Auterion / px4-jsbsim-bridge

JSBSim bridge for PX4 SITL/HITL simulations
BSD 3-Clause "New" or "Revised" License
26 stars 38 forks source link

Enable user defined GPS Parameters from JSBSim #19

Closed mvacanti closed 3 years ago

mvacanti commented 3 years ago

@Jaeyoung-Lim this proposal moves enables the user to map custom JSBSim parameters via the bridge configuration file and JSBSim system definition file. When configuration definition provided it will default to the "standard" inputs / behaviors.

mvacanti commented 3 years ago

@Jaeyoung-Lim when I include those files in my fork I get the following error when I attempt to push to origin - thoughts?:

error: failed to push some refs to 'https://github.com/mvacanti/px4-jsbsim-bridge.git'
To https://github.com/mvacanti/px4-jsbsim-bridge.git
!   refs/heads/pr-gps-xml-param-mapping:refs/heads/pr-gps-xml-param-mapping [remote rejected] (refusing to allow an OAuth App to create or update workflow `.github/workflows/firmware_build_test.yml` without `workflow` scope)
Done
Jaeyoung-Lim commented 3 years ago

@mvacanti What are you using to push the code? have you tried using a normal command line command? (referring to the OAuth app that is mentioned in the error)

mvacanti commented 3 years ago

@Jaeyoung-Lim - That solved it and I reverted the changes. IDE was not configured correctly.

mvacanti commented 3 years ago

@Jaeyoung-Lim Thanks! This is only beginning to scratch the surface of what can be done.

Do you think it would make sense using the "system" as default for all sensor plugins? I think this is straight forward for the gps plugin, since it didn't have a noise model before.

I do not think we should force all models to use the systems definition (this would create conflicts with the Rascal for example) but I do believe that we should create default jsbsim systems files for all sensors (imu, baro, airspeed, etc).

What do you think we should do to sensors that already have noise models already integrated?

My general opinion is that sensors that already have built-in noise models already should stay that way as it allows a base configuration that works out of the box. I have started on the IMU conversion and I see three valid use cases:

  1. Built-in noise model for models such as the Rascal. It works out of the box without modification.
  2. Runtime variable noise model for advanced configuration and system testing a. User "zeros out" built-in noise model via config file b. User configures JSBSim system files and script as desired
  3. Combination of systems file and built-in noise model. a. This allows user to "fail" the built-in sensor without adding new noise
Jaeyoung-Lim commented 3 years ago

@mvacanti Thanks for the feedback. I think this looks good as it is for now then