-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[settings] Add getters #3307
Comments
I like this idea :) |
Hi @Fabien-B, this looks nice and very coherent. I'm just missing which situation is not well handled with the current setup? |
Hi, there are multiple times where a variable is not needed, but you need to define it anyway because of how settings works currently. paparazzi/sw/airborne/modules/computer_vision/tag_tracking.h Lines 66 to 76 in 36828c0
Here the variable is useless: paparazzi/sw/airborne/modules/loggers/sdlog_chibios/usbStorage.c Lines 171 to 177 in 36828c0
And in a more general way, the current system forces us to have global variables everywhere, which is a bad design choice. We can't change everything in paparazzi at once, but a change like this can allow us to slowly shift towards cleaner code. |
Thanks a lot. Great suggestion. I'll investigate which unnecessary variables we introduced once this is merged. |
In the settings, we have the
handler
that allows to call a function to set the value.We need the same mecanism to get the value: a getter.
This getter should return a
float
, to be consistent with the current system.Current situation
Let's take an example with this setting:
_value = autopilot.kill_throttle;
(line 4) should be removed: If there is a handler, the value should not be forcefully changed after calling the handler.Proposed new behavior
The setting could be changed like this:
It would result in this generated code:
Defining the getter like this
getter="getKillThrottle()"
allows to specify either a function call, or a global variable like in the current situation.The
name
attribute would be added to define the name of the setting.shortname
would be deprecated in favor ofname
.The name resolution order would be:
name
,shortname
(with deprecated warning),var
.XML valid cases
The following should Fail at compilation, because
var
would be unused:The text was updated successfully, but these errors were encountered: