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

FIX: fixed-wing airspeed and wind estimation problem in SITL #24437

Closed
wants to merge 2 commits into from

Conversation

TigerWuu
Copy link

@TigerWuu TigerWuu commented Mar 3, 2025

Solved Problem

Fixes #23756

Solution

Changelog Entry

N/A

Alternatives

N/A

Test coverage

N/A

Context

N/A

@TigerWuu TigerWuu marked this pull request as ready for review March 3, 2025 08:00
@dakejahl
Copy link
Contributor

dakejahl commented Mar 3, 2025

Instead of adding a uORB message and plumbing this into SensorAirspeedSim you should instead publish differential pressure directly from the GZBridge. You should then disable SENS_EN_ARSPDSIM when using the wind plugin. Is the plugin added to the world file? I guess we will have to check in the startup script if we're running ionic and disable SENS_EN_ARSPDSIM from there, that way it happens automatically.

@TigerWuu
Copy link
Author

TigerWuu commented Mar 4, 2025

Thanks for your advice.

Is the plugin added to the world file?

This plugin has not been added to any world files so far. This plugin only works along with the <wind> tag. e.g. in windy.sdf

Instead of adding a uORB message and plumbing this into SensorAirspeedSim you should instead publish differential pressure directly from the GZBridge.

I was wondering if it will cause ambiguity in GZBridge. As far as I know, GZBridge is only in charge of transmitting the information from Gazebo to PX4. Publishing the differential pressure from GZBridge represents extra calculations should be conducted in GZBridge, i.e.,

Vector3f Va_vec = local_velocity - wind_velocity; 
float Va = Va_vec.norm();
diff_pressure = sign(Va) * 0.005f * air_density  * Va * Va + diff_pressure_noise;

On the other hand, wouldn't it be unintuitive to disable SENS_EN_ARSPDSIM when doing an airspeed simulation?

@dakejahl
Copy link
Contributor

dakejahl commented Mar 4, 2025

This plugin has not been added to any world files so far. This plugin only works along with the tag. e.g. in windy.sdf

We can add it to our PX4 plugins list once #24441 is merged

As far as I know, GZBridge is only in charge of transmitting the information from Gazebo to PX4. Publishing the differential pressure from GZBridge represents extra calculations should be conducted in GZBridge

Differential pressure can be calculated directly within the GZBridge, so we should do it there instead of coupling it to another module.

On the other hand, wouldn't it be unintuitive to disable SENS_EN_ARSPDSIM when doing an airspeed simulation?

Yes you're right it is unintuitive, the reason it is done this way is that historically there have been some sensors that we have to "fake" with the Sensor<SensorName>Sim modules. These modules are enabled in the airframe files. Airspeed and magnetometer are the last 2 to eliminate for gz so this gets us halfway there! https://github.com/PX4/PX4-Autopilot/blob/main/ROMFS/px4fmu_common/init.d-posix/airframes/4004_gz_standard_vtol#L16-L17

In reality it is SIH that needs these "fake" sensors, since SIH does not have a physics engine to provide them.
https://docs.px4.io/main/en/sim_sih/index.html

@TigerWuu
Copy link
Author

TigerWuu commented Mar 4, 2025

This plugin has not been added to any world files so far. This plugin only works along with the tag. e.g. in windy.sdf
We can add it to our PX4 plugins list once #24441 is merged

Awesome!

On the other hand, wouldn't it be unintuitive to disable SENS_EN_ARSPDSIM when doing an airspeed simulation?

Yes you're right it is unintuitive, the reason it is done this way is that historically there have been some sensors that we have to "fake" with the Sensor<SensorName>Sim modules. These modules are enabled in the airframe files. Airspeed and magnetometer are the last 2 to eliminate for gz so this gets us halfway there! https://github.com/PX4/PX4-Autopilot/blob/main/ROMFS/px4fmu_common/init.d-posix/airframes/4004_gz_standard_vtol#L16-L17

In reality it is SIH that needs these "fake" sensors, since SIH does not have a physics engine to provide them. https://docs.px4.io/main/en/sim_sih/index.html

Okay, I see. Cool!

As far as I know, GZBridge is only in charge of transmitting the information from Gazebo to PX4. Publishing the differential pressure from GZBridge represents extra calculations should be conducted in GZBridge

Differential pressure can be calculated directly within the GZBridge, so we should do it there instead of coupling it to another module.

In that case, I would like to try if it is possible to switch between body velocity or true airspeed to calculate the differential_pressure in GZBridge. It looks like we had better remove the usage of Sensor<SensorName>Sim modules in SITL, right?

@dakejahl
Copy link
Contributor

dakejahl commented Mar 4, 2025

You can use the models groundtruth velocity and the wind velocity in the GZBridge::windCallback to calculate and publish differential pressure.

It looks like we had better remove the usage of SensorSim modules in SITL, right?

Yes we should remove it from all of the gz_* models airframe files
https://github.com/PX4/PX4-Autopilot/tree/main/ROMFS/px4fmu_common/init.d-posix/airframes
for example
https://github.com/PX4/PX4-Autopilot/blob/main/ROMFS/px4fmu_common/init.d-posix/airframes/4004_gz_standard_vtol#L17

But that means we need to add wind to all of the worlds (all zeroes, except for the windy world) to ensure the callback runs so that we always get an airspeed published.

@TigerWuu
Copy link
Author

TigerWuu commented Mar 4, 2025

But that means we need to add wind to all of the worlds (all zeroes, except for the windy world) to ensure the callback runs so that we always get an airspeed published.

I was wondering if we could publish the differential_pressure in poseInfoCallback so that I can use the information of body velocity in case we don't have the wind information to calculate the differential_pressure. This way, we don't have to add a wind plugin to all the world files. what do you think?

@dakejahl
Copy link
Contributor

dakejahl commented Mar 4, 2025

Yes that makes more sense!

@TigerWuu
Copy link
Author

TigerWuu commented Mar 5, 2025

Yes that makes more sense!

Done! I will close this PR and reopen another one.

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.

[Bug] fixed-wing airspeed and wind estimation problem in SITL
2 participants