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

Parameter system enhancement #14363

Closed
wants to merge 18 commits into from
Closed

Parameter system enhancement #14363

wants to merge 18 commits into from

Conversation

RomanBapst
Copy link
Contributor

This pull request brings the following enhancements to the PX4 parameter system:

Short summary

  • ability to store and fetch the true default parameter value which may stem from the generic defaults, board config, vehicle_type config (mc, fw, vtol) or the airframe config.
  • extracted parameters from autostart scripts and put them into dedicated parameter files, which are converted to bson format at build time
  • made the SYS_AUTOCOFNIG mechanism obsolete

Detailed Summary

  • Introduction of param load-as-default param_file. This new command reads parameter values from a file and stores them internally as the default value for that parameter. This default value is not the same as the actual parameter value, the two things are stored separately. If that parameter has not been modified before (e.g. it does not exist in the list of params stored in FRAM) it will also set the value of the parameter accordingly.
    This command is used at each place where there used to be the AUTOCNF section.
  • In order to make use of the above command all the parameters being set in the autoconfig sections had to be moved to appropriate files which are now used to generate bson format parameter files.
    This brings the advantage that the default board-, vehicle_type- and airframe config parameters are now accessible as files.
  • order of loading default parameters: The current order of loading default params is:
  1. board defaults
  2. vehicle type defaults
  3. airframe config default

I created a spreadsheets which shows how the actual- and default value of a specific parameter changes for different scenarios.
https://docs.google.com/spreadsheets/d/1RVHwuwpJRBwl1YKYZ88FEJS0VzTK17DghgDgVqUuJwc/edit?usp=sharing

Open points for discussion

  • there are parameters which are set in various autostart scripts but outside of any AUTOCNF section. Those parameter will appear as modified parameters.
  • calibration parameters will also show up as modified parameters

@dagar
Copy link
Member

dagar commented Mar 12, 2020

extracted parameters from autostart scripts and put them into dedicated parameter files, which are converted to bson format at build time

Is there any actual benefit to keeping this as bson other than import simplicity? Wondering if we should do it directly in a human readable format and leave the possibility of loading from the sd card open.

@dagar dagar added this to the Release v1.12.0 milestone Mar 12, 2020
@dagar
Copy link
Member

dagar commented Mar 12, 2020

I'm wondering if the safest thing to do would be to load the defaults as soon as possible right next to the param load.

https://github.com/PX4/Firmware/blob/9162c8b196785e7452c928e199118b441e68f320/ROMFS/px4fmu_common/init.d/rcS#L139-L146

What do you think?

param_wbuf_s *s = param_find_changed(param);

if (s == nullptr) {
// // don't store new value if default
// if (val_is_default) {
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 this commented?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dagar I was thinking about one case where this might lead to undesired behavior but thinking about it again I think it's ok. I will add this again.

@BazookaJoe1900
Copy link
Member

BazookaJoe1900 commented Mar 12, 2020

I liked the this approach! and have some small notes/questions:

  • what about renaming the airframe defaults files from 10015.params to 10015_default.params? so it can be clearer its defaults?
  • there are many return 0/-1. that can be changed to return PX4_OK/PX4_ERROR
  • is there a way to avoid the 'static bool set_as_default' and in some way to move it as argument to 'param_import_callback'? its just confuses...

@hamishwillee
Copy link
Contributor

@RomanBapst @dagar Does this change put us in a position where parameters can be translated?
Specifically can we round-trip the generated files through crowdin and use them to export parameter docs and whatever is used by QGC?

@RomanBapst
Copy link
Contributor Author

RomanBapst commented Mar 16, 2020

I'm wondering if the safest thing to do would be to load the defaults as soon as possible right next to the param load.

https://github.com/PX4/Firmware/blob/9162c8b196785e7452c928e199118b441e68f320/ROMFS/px4fmu_common/init.d/rcS#L139-L146

What do you think?

Yes, I think we can do that. Let me change and do some tests.

@RomanBapst
Copy link
Contributor Author

@BazookaJoe1900

what about renaming the airframe defaults files from 10015.params to 10015_default.params? so it can be clearer its defaults?

Yes, I think we can do that.

there are many return 0/-1. that can be changed to return PX4_OK/PX4_ERROR

Ok, will check.

is there a way to avoid the 'static bool set_as_default' and in some way to move it as argument to 'param_import_callback'? its just confuses...

Possibly, I agree that the current solution is not the best.

@RomanBapst
Copy link
Contributor Author

@hamishwillee

Does this change put us in a position where parameters can be translated?
Specifically can we round-trip the generated files through crowdin and use them to export parameter docs and whatever is used by QGC?

I can explain to you what we have right now. The generic PX4 defaults are still and only accessible via the Parameters.xml file.
For each board (fmuv2, fmuv3, ...) there is a parameter file holding the board defaults.
There are 3 parameters files holding default values for each multicopter, fixed wing and vtol.
For each airframe there is a paramter file.

The question is in which format do you want to have them. Before build time they are available in a human readable format and after build time they are available in bson format (the format which the PX4 paramter system uses to load and save params.)

- added param import-as-default command
- support storing a default parameter value set at run-time

Signed-off-by: RomanBapst <[email protected]>
human readable parameter files

Signed-off-by: RomanBapst <[email protected]>
Signed-off-by: RomanBapst <[email protected]>
Signed-off-by: RomanBapst <[email protected]>
Signed-off-by: RomanBapst <[email protected]>
@bys1123
Copy link
Contributor

bys1123 commented Apr 18, 2020

Is this PR can also save flash memory?

@hamishwillee
Copy link
Contributor

FYI We are in the process (see #15389) of adding support for GCS to query a vehicle for metadata and other information. The GCS requests metadata of a particular type and COMPONENT_INFORMATION message is returned with URI (HTTP or mavftp) of file containing that data in .json.gz format.

There has been some discussion that it would be useful to be able to provision parameters as a file via MAVFTP and for GCS request them - a compressed .json.bz file likely to be more far efficient to download via MAVFTP than via the current parameter format.

Don't know if this would impact how you implement this.

FWIW Very cool that the defaults can be supplied as a file. Allowing these to be on SD card or in firmware and have the system choose the SD card preferentially is a good model. Having the default be factory updatable via mavlink even cooler.
Any reason you can't have the file format as either json or json.gz and the system accepts either (or json and bson)?

@stale stale bot removed the stale label Jul 28, 2020
@PX4 PX4 deleted a comment from stale bot Aug 25, 2020
@dagar
Copy link
Member

dagar commented Aug 25, 2020

I'd like to get this moving again. Can we establish a plain text parameter file format (or use QGCs) and skip the build time bson generation?

My goal is that ultimately an airframe is nothing more than a set of parameters. Some of those options can live directly in the code base for convenience, but for the majority of users it should be sufficient to have a simple parameter file they can save/load from QGC.

@dagar dagar self-assigned this Aug 25, 2020
@dagar
Copy link
Member

dagar commented Aug 25, 2020

Why do we need both import-as-default and import-airframe? Isn't it a runtime default regardless?

@dagar
Copy link
Member

dagar commented Aug 25, 2020

Needs to be rebased.

@dagar
Copy link
Member

dagar commented Aug 25, 2020

Alternatively, we could get the core parameter system changes in, then incrementally start transitioning the existing AUTOCNF scripting.

@stale
Copy link

stale bot commented Dec 25, 2020

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@mrpollo
Copy link
Contributor

mrpollo commented Feb 3, 2021

Feb 3, 2021, Dev Call update: The plan going forward is to split this PR in two, one for the Param architecture, and the second for the parameter per board updates which can happen over time.

@mrpollo
Copy link
Contributor

mrpollo commented Feb 10, 2021

Continued in #16796

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.

6 participants