Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gz simulation adding tiltrotor airframe #24028

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

Perrrewi
Copy link
Contributor

@Perrrewi Perrrewi commented Nov 22, 2024

Solved Problem

Moving the tiltrotor from gazebo classic to gz sim as gazebo garden will be depreciated by the end of nov 24

Solution

Adding an airframe and fixed the bug in gz-bridge. PX4 sends an output value from 0 to 1000 of the servo and the GZbridge normalized it to be within +-1. The problem was that gazebo needs a servo input of angles in radian which meant the currently servos only works in the range +-1 rad.

Test coverage

Tested with QGC

Context

Test: Toggling the actuator testing slider with max output value 1000 while gazebo angle limitations were set to 90 deg

Before updating GZMixingInterfaceServo.cpp:
Screenshot from 2024-11-27 16-42-11
Screenshot from 2024-11-27 17-09-25

After updating GZMixingInterfaceServo.cpp:
Screenshot from 2024-11-27 16-41-02

Screenshot from 2024-11-27 17-07-45

@Perrrewi Perrrewi self-assigned this Nov 22, 2024
@Perrrewi Perrrewi mentioned this pull request Nov 22, 2024
30 tasks
Copy link

github-actions bot commented Nov 22, 2024

🔎 FLASH Analysis

px4_fmu-v5x [Total VM Diff: 8 byte (0 %)]
    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%      +8  +0.0%      +8    .text
  +0.0%      +5  +0.0%      +5    [section .text]
  +0.2%      +3  +0.2%      +3    ../../src/systemcmds/ver/ver.cpp
+0.0%     +56  [ = ]       0    .debug_abbrev
   +11%     +56  [ = ]       0    ../../src/lib/version/version.c
-0.0%      -8  [ = ]       0    .debug_aranges
  -5.0%      -8  [ = ]       0    ../../src/lib/version/version.c
-0.0%      -4  [ = ]       0    .debug_info
  -0.2%      -4  [ = ]       0    ../../src/lib/version/version.c
-0.0%     -20  [ = ]       0    .debug_line
  -1.3%     -25  [ = ]       0    ../../src/lib/version/version.c
  +0.4%      +5  [ = ]       0    task/task_cancelpt.c
-0.0%      -8  [ = ]       0    .debug_ranges
  -2.6%      -8  [ = ]       0    ../../src/lib/version/version.c
-0.0%      -8  [ = ]       0    [Unmapped]
+0.0%     +16  +0.0%      +8    TOTAL

px4_fmu-v6x [Total VM Diff: 0 byte (0 %)]
    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%     +56  [ = ]       0    .debug_abbrev
   +11%     +56  [ = ]       0    ../../src/lib/version/version.c
-0.0%      -8  [ = ]       0    .debug_aranges
  -5.0%      -8  [ = ]       0    ../../src/lib/version/version.c
-0.0%      -4  [ = ]       0    .debug_info
  -0.2%      -4  [ = ]       0    ../../src/lib/version/version.c
-0.0%     -28  [ = ]       0    .debug_line
  -1.3%     -25  [ = ]       0    ../../src/lib/version/version.c
  -0.3%      -3  [ = ]       0    task/task_cancelpt.c
-0.0%      -8  [ = ]       0    .debug_ranges
  -2.6%      -8  [ = ]       0    ../../src/lib/version/version.c
+0.0%      +8  [ = ]       0    TOTAL

Updated: 2024-12-16T13:06:17

@Perrrewi Perrrewi force-pushed the pr-add-quadtailsitter branch from 8d2185c to 827ed69 Compare November 26, 2024 08:32
@Perrrewi Perrrewi force-pushed the pr-gz-sim-tiltrotor branch from c8bd4a4 to d223a8a Compare November 26, 2024 11:04
@Perrrewi Perrrewi changed the title Pr gz sim tiltrotor Draft: Pr gz sim tiltrotor Nov 26, 2024
@Perrrewi Perrrewi force-pushed the pr-gz-sim-tiltrotor branch from d223a8a to 08494be Compare November 26, 2024 16:13
@Perrrewi Perrrewi changed the title Draft: Pr gz sim tiltrotor Pr gz sim tiltrotor Nov 27, 2024
@Perrrewi Perrrewi force-pushed the pr-gz-sim-tiltrotor branch from bf264cf to 66ab707 Compare November 28, 2024 17:53
@Perrrewi Perrrewi force-pushed the pr-add-quadtailsitter branch from 827ed69 to 81fc89a Compare November 29, 2024 08:04
@Perrrewi Perrrewi force-pushed the pr-gz-sim-tiltrotor branch 2 times, most recently from b561a74 to 4ec9990 Compare November 29, 2024 09:10
@Perrrewi Perrrewi force-pushed the pr-add-quadtailsitter branch from 81fc89a to f08beb8 Compare December 2, 2024 09:53
@Perrrewi Perrrewi force-pushed the pr-gz-sim-tiltrotor branch from 4ec9990 to d47d5f8 Compare December 2, 2024 10:10
Base automatically changed from pr-add-quadtailsitter to main December 2, 2024 16:27
@Perrrewi Perrrewi force-pushed the pr-gz-sim-tiltrotor branch from d47d5f8 to ab93cd7 Compare December 2, 2024 16:54
@@ -64,8 +64,8 @@ bool GZMixingInterfaceServo::updateOutputs(bool stop_motors, uint16_t outputs[MA
for (auto &servo_pub : _servos_pub) {
if (_mixing_output.isFunctionSet(i)) {
gz::msgs::Double servo_output;
///TODO: Normalize output data
double output = (outputs[i] - 500) / 500.0;
double output = math::radians((double)outputs[i] / 10. - 180.);;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you assuming +1 is always 180 degrees? This is not true in general. We need a parameter defined which maps the normalized angles to servo angles. (part of the reason why the +1 radian was left like that)

Copy link
Contributor Author

@Perrrewi Perrrewi Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that we only use angles within +-180 deg and the output default values has been changed.

So if we want a zero symmetrical range of +-45 deg, then output max is set to 2250, and min to 1350.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Perrrewi I am not talking about the limits, but rather that the angles need to be configurable. For example, tiltrotors normally operate the servo angles 0-90 deg, which you could do with +-45, but it would be great if we can actually do it properly and configure it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Jaeyoung-Lim I made an update with angle min max as parameters.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried adding the parameters on the control surface parameters CA_SV_CS in the control allocator, but then some targets got out of flash memory.

For now I cannot see a good solution before changing the types from uints to floats as proposed here: #22488

@Perrrewi Perrrewi force-pushed the pr-gz-sim-tiltrotor branch 3 times, most recently from 0c60b0b to 642f09f Compare December 4, 2024 15:18
@Perrrewi Perrrewi force-pushed the pr-gz-sim-tiltrotor branch 2 times, most recently from 33d2023 to 94da813 Compare December 11, 2024 17:15
@Claudio-Chies Claudio-Chies self-requested a review December 12, 2024 07:09
@Perrrewi Perrrewi changed the title Pr gz sim tiltrotor gz simulation adding tiltrotor airframe Dec 12, 2024
Copy link
Contributor

@Claudio-Chies Claudio-Chies left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I assume this is what jaey meant with the parameter based implementation,
left just a few minor improvements, to avoid issues with other airframe implementations if accessing out of range indices.

@Perrrewi Perrrewi force-pushed the pr-gz-sim-tiltrotor branch from 94da813 to 639b005 Compare December 16, 2024 10:58
@Perrrewi Perrrewi force-pushed the pr-gz-sim-tiltrotor branch from 639b005 to 6b83045 Compare December 16, 2024 13:01
disarmed: { min: 0, max: 3600, default: 1800 }
min: { min: 0, max: 3600, default: 1350 }
max: { min: 0, max: 3600, default: 2250 }
failsafe: { min: 0, max: 3600 }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where do these numbers come from? Can we normalize it for simulation?

@@ -33,6 +33,9 @@

#include "GZMixingInterfaceServo.hpp"

#define SERVO_OUTPUT_SCALING (10.)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 10?

@@ -33,6 +33,9 @@

#include "GZMixingInterfaceServo.hpp"

#define SERVO_OUTPUT_SCALING (10.)
#define SERVO_OUTPUT_OFFSET (180.)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it necessary to have this as an ifdef?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants