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

Air data improvements and conversion to module #853

Merged
merged 13 commits into from Oct 28, 2014
Merged

Conversation

flixr
Copy link
Member

@flixr flixr commented Oct 11, 2014

Converts air_data to a module and adds:

  • math functions for QNH, etc. from Qnh #837
  • subscribe to BARO_DIFF via ABI
  • subscribe to TEMPERATURE via ABI
  • calculate airspeed from differential pressure if enabled (default)
  • calculate QNH from abs pressure and altitude (enabled by default)
  • calculate AMSL from baro pressure and QNH (disabled by default)
  • calculate true airspeed (TAS) from equivalent airspeed (EAS) using a tas_factor (with default to 1.0)
  • calculate tas_factor from ISA model and current pressure and temperature (enabled by default)

I think this is a better place to handle QNH than the qnh.xml module from #837

QNH and TAS not tested in real flight so far...

@flixr
Copy link
Member Author

flixr commented Oct 20, 2014

@gautierhattenberger what do you think? Good to merge?
The QNH stuff is not really tested, but I would like to get the ABI things in master asap.
The only impact merging this pull request should have on existing/working things is that if you don't load the air_data.xml module, you won't get BARO_RAW messages....

@gautierhattenberger
Copy link
Member

I think it is fine to merge, but maybe adding a warning telling to load the module would be nice, at least for some time, what do you think ?

@flixr
Copy link
Member Author

flixr commented Oct 20, 2014

Warning to load the module for which cases? For the fw/rc firmware in general so you can get BARO_RAW?

@gautierhattenberger
Copy link
Member

of course it is not the best to have the warning in the general case, but it can be temporary so people are not surprised to "loose" the BARO_RAW message.

@flixr
Copy link
Member Author

flixr commented Oct 20, 2014

Hm... not a big fan of such a general warning as everyone will get this warning (even if they don't have a baro or never use/need the BARO_RAW message)... I would rather write an email on the mailing list about these changes.

But also wondering about the message name: maybe BARO_RAW should rather be BARO_SCALED, since it is really the scaled value (in Pa) and not some baro specific raw sensor value.

@gautierhattenberger
Copy link
Member

I guess it is just a few more question on the ML if people miss the email.

For the name, it use to be a raw value, but since it is not anymore, I guess we can change it now.

@flixr
Copy link
Member Author

flixr commented Oct 20, 2014

Ok... just add whatever warning you think is best...

Regarding the BARO_RAW message: not sure if it actually makes sense to just rename it to BARO_SCALED, since not that many people actually have a differential pressure sensor connected...
So maybe makes more sense to rather create something like an AIR_DATA message that also has the temperature, qnh, amsl_baro?
But maybe that is something that needs more discussion and should be done in a later pull request...

@flixr
Copy link
Member Author

flixr commented Oct 22, 2014

Added an AIR_DATA message, would actually be in favor of dropping the BARO_RAW message (since it is not raw, you have the sensor specific messages for that, and diff is not used in most cases anyway).

- removed airspeed_scale
- true airspeed (TAS) factor calculated from pressure and temp
  - if that is not available, tas_factor can be set manually (and could be used to replace airspeed_scale
so you can let it calc the airspeed without pushing it to the state interface,
e.g. to use a different airspeed sensor for state interface and hence control
@flixr flixr mentioned this pull request Oct 23, 2014
Closed
@flixr
Copy link
Member Author

flixr commented Oct 23, 2014

@gautierhattenberger I would also drop the aoa and sideslip, it is not used anyway...

I guess we can keep the AIR_DATA name as it is short... although atmospheric data would be a bit more precise.

@gautierhattenberger
Copy link
Member

It is not using aoa and sideslip but it should. I just didn't make it yet, but the AOA module should also publish an ABI message and let air_data update the state interface. It would be consistent with other sensors.

About the name itself, on big aircraft, this system is called Air Data Computer, but the name ADC was not an option here !

@flixr
Copy link
Member Author

flixr commented Oct 26, 2014

@gautierhattenberger ok, sounds good..

@flixr
Copy link
Member Author

flixr commented Oct 27, 2014

@gautierhattenberger, unless you want to add something here now, I would merge it as is.
Regarding the BARO_RAW message, I propose to remove it anyway, so a warning if you don't have this module loaded is a bit useless IMHO...

@gautierhattenberger
Copy link
Member

Ok to merge like this.

The only thing I can see is to add a binding to an airspeed message, in case a sensor provides directly this value. Since we don't have such thing yet, it is not really urgent...

@flixr
Copy link
Member Author

flixr commented Oct 28, 2014

Actually all airspeed sensor modules currently have the option to directly compute and set the airspeed and e.g. airspeed_ets doesn't output differential pressure and instead only the raw value, offset and airspeed.

But still merging this as binding to an airspeed ABI message can still be done later.

flixr added a commit that referenced this pull request Oct 28, 2014
Converts air_data to a module and adds:
- math functions for QNH, etc. from #837 
- subscribe to BARO_DIFF via ABI
- subscribe to TEMPERATURE via ABI
- calculate airspeed from differential pressure if enabled (default)
- calculate QNH from abs pressure and altitude (enabled by default)
- calculate AMSL from baro pressure and QNH (disabled by default)
- calculate true airspeed (TAS) from equivalent airspeed (EAS) using a `tas_factor` (with default to 1.0)
- calculate `tas_factor` from ISA model and current pressure and temperature (enabled by default)
- send new AIR_DATA and AMSL messages
@flixr flixr merged commit 93b9546 into master Oct 28, 2014
@flixr flixr deleted the air_data_improvements branch October 28, 2014 21:33
@flixr flixr added this to the v5.4 milestone Nov 24, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants