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

Clarify mapping of physical batteries to battery slots #13033

Open
AmeliaEScott opened this issue Sep 26, 2019 · 6 comments
Open

Clarify mapping of physical batteries to battery slots #13033

AmeliaEScott opened this issue Sep 26, 2019 · 6 comments

Comments

@AmeliaEScott
Copy link
Contributor

AmeliaEScott commented Sep 26, 2019

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 and INA226_BAT2_SLOT.

Pros:

  • Simple to implement
  • Configurable

Cons:

  • Adds more parameters
  • Possibly confusing to end users. Changing a *_SLOT parameter will also change which parameters configure that battery. E.g., a battery on slot 0 is configured by BAT1_* parameters. But moving it to Slot 1 makes it be configured by BAT2_* 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:

  • Simple for end users
  • No further configuration necessary

Cons:

  • There needs to be some mechanism for letting the user know that, for example, the first Digital battery is calibrated by BAT2_* parameters because it is in Slot 2
@AmeliaEScott
Copy link
Contributor Author

FYI @dagar @hamishwillee @julianoes

@julianoes
Copy link
Contributor

@dagar could we build this in a similar way to the params for the serial configuration?

@hamishwillee
Copy link
Contributor

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 :-)

@dagar
Copy link
Member

dagar commented Oct 4, 2019

@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.

@dagar
Copy link
Member

dagar commented Oct 4, 2019

@dagar could we build this in a similar way to the params for the serial configuration?

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.

@stale
Copy link

stale bot commented Jan 2, 2020

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

@stale stale bot added the stale label Jan 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants