reduce the use of param_find("XXX") #13089
Conversation
…ams::XXX) that allows build time error if parameter is no in a version
|
We actually need to be more careful here than it first appeared. One difference is the param_find() also marks the parameter active (which then syncs to QGC). |
|
I must say that in general I don't like the idea of "active" parameter. I think that either there is a parameter on a system or not. the advantage of keep using 'active' is that downloading parameters might take a bit faster. |
Yes it makes downloading faster, but it's more fundamental in that it's really what makes the system scalable from a user perspective. We can have release binaries with all kinds of controllers and drivers, but the user is only going to see a relevant subset for configuration. The pattern that should be enforced is that a module should be marking all of its parameters active when it starts. Virtually all the problems have come from doing this lazily or incorrectly. |
|
Seconding here - all what Daniel says its correct. The system is only tractable with this enabled. The same way that C++ requires correct initialization of pointers we require correct initialization of parameters. You can argue that the programming environment should address both, but then really the right starting point is to replace C++ with something a lot more defined, and that is what most people are not debating. If you go down that path you ought to use purely compile-time configurations and binaries for every single airframe type - and yet PX4 has won against these legacy architectures because it creates an ecosystem that is able to generalize more and move faster. |
|
@dagar @LorenzMeier thanks for the replay, I appreciate how you look of things, and agree. My case study is after I removed most of the modules that are not relevant to me... still, about this pr. there is still need to minimize the use of param_find("XXX"), and use hardcoded parameter name. so how about using param_set_used() in param_handle()? |
How about adding a param_find that only takes enum and marks it active? It would have to be c++ only similar to this. Then the replacement would be even cleaner.
|
|
I am not sure that 'param_find()' will be the right term, you don't really search for it, you know it exist. |
|
seems to be irrelevant, closing |
reduce the use of param_find("XXX") in favor to param_handle(px4::params::XXX)
The pr is replacement to #13076
In case parameter is not in a version, there will be build time error.
My method was to set all parameters to use param_handle(px4::params::XXX) and then find what is not being built.
that mean that every place that there you can still find a param_find("XXX") some build/s been broken.
That mean that those places basically need to be fixed.
on some places the error seems as more critical then others, for example building px4_fmu-v2_test need the parameter FW_PSP_OFF but it doesn't exists. and later there is no check if it exists or not. that mean that in _params_handles_standard.pitch_setpoint_offset there is PARAM_INVALID and later there is garbage when trying to fetch the parameter