-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
Clarify mapping of physical batteries to battery slots #13033
Comments
@dagar could we build this in a similar way to the params for the serial configuration? |
FWIW mavlink/mavlink#1182 did not actually get accepted yet. Too many opinions for consensus and no real experts to break deadlock. That said, it is pretty clear that there needs to be a way for a GCS to get the parameters associated with a particular battery. This doesn't need to be complicated though - it could be as simple as a battery instance having a number "n" and all parameters for that battery sharing that instance number. @julianoes If that is what you meant by the serial configuration stuff, then yes :-) |
@ItsTimmy I'm not sure that I fully understand the problem. For the majority of usage isn't the core information already known per board? For example fmu-v5 having 2 analog power modules, fmu-v5x having 2 digital power modules. Supporting arbitrary runtime configurations would be nice, but it's not necessarily a real problem for users (and products). Isn't is also going to be a different set of parameters per battery type? If it is truly necessary to uniquely identify them then something similar to the way we do it now for sensors with device ids might work. This might be easier to discuss on a call. |
If you're referring to instances of parameters, then possibly. Doing that for serial was a big improvement, but it generates quite a lot of nearly identical code for the parameters and startup. It also means having to reboot when changing the value, which really doesn't need to be the case for most. All that is to say, I think we might be able to do better by internalizing the concept of parameter instance. For example instead of literally duplicating BAT_N_CELLS as BAT1_N_CELLS, BAT2_N_CELLS, BAT3_N_CELLS, etc we have a single BAT_N_CELLS_x, with a single set of static metadata. At runtime you can have as many instances as needed. Then instead of generating the scripting to pass in generated parameter strings to a module we could just read them internally (extending the param api slightly). I'll start hacking this out for further discussion at some point. |
This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions. |
Describe problem solved by the proposed feature
In mavlink/mavlink#1182, the Mavlink spec will be changed to clarify how multiple batteries should be reported. There will be a fixed number of battery slots, numbered sequentially from 0. Any given physical battery should consistently map to the same slot for the lifetime of the vehicle.
In #12551, this spec is followed, but only 1 or 2 analog power modules are supported. #12673 adds support for digital I2C power modules. In order to finish #12673, there needs to be a mechanism for mapping each power module to a battery slot.
Describe your preferred solution
The driver/module for each type of battery can add a set of parameters for determining the slot number for its own battery(s). For example, the INA226 driver (From #12673) could add two parameters:
INA226_BAT1_SLOT
andINA226_BAT2_SLOT
.Pros:
Cons:
*_SLOT
parameter will also change which parameters configure that battery. E.g., a battery on slot 0 is configured byBAT1_*
parameters. But moving it to Slot 1 makes it be configured byBAT2_*
parameters.Describe possible alternatives
Determine some fixed set of battery types that are supported, and assign them to a list. For example, if a vehicle supports 2 analog power modules and 2 digital, set the list to:
[Analog0, Analog1, Digital0, Digital1]
The battery slot is just the index in this list.
Pros:
Cons:
BAT2_*
parameters because it is in Slot 2The text was updated successfully, but these errors were encountered: