[WIP] Add functions for interacting with the primary battery configuration

[marmistrz]

This pull request is intended to enable the control of the Primary Battery Charge Configuration from libsmbios, similar to what --PrimaryBattChargeCfg from cctk does. These tokens are probably Dell-specific and will possibly allow advanced power management integration on Dell laptops using TLP. (Currently TLP only supports this feature on Thinkpads)

Things to be done before merging

• [x] deactivate the battery modes in set_charging_mode
• [x] connect the newly added functions to the CLI parser
• [x] validate the charge interval before writing it

I have encountered two problems while working on this PR:

• how should I properly deactivate a token from the TokenTable? smbios_token.Token only exposes a method activate and no deactivate.
• there are two available tokens for the custom charge (cf. class ChargingMode). Which of them should be used?

[superm1]

how should I properly deactivate a token from the TokenTable? smbios_token.Token only exposes a method activate and no deactivate.

Typically is usually an opposite token for disable. You should "enable" that token to cause a disable.

there are two available tokens for the custom charge (cf. class ChargingMode). Which of them should be used?

You should use 0x343.

[superm1]

(FYI, CI has a known failure with CentOS right now, haven't had time to debug it yet)

[marmistrz]

Typically is usually an opposite token for disable. You should "enable" that token to cause a disable.

Does it mean that, for instance, if I enable 0x0342, then 0x0343 will automatically get disabled?

[superm1]

0x342 is setting "Adaptive Charge" 0x343 is setting "Custom Charge"

Only one can be set, so yes once you set 0x343 to be activated 0x342 will be deactivated.

[marmistrz]

Let me know if you're ok with this CLI:

  --get-charging-cfg    Get the current Primary Battery Charge Configuration
  --set-charging-mode=<MODE>
                        Set the current Primary Battery Charge Configuration.
                        Valid choices are: ['primarily_ac', 'adaptive',
                        'custom', 'standard', 'express']
  --set-custom-charge-interval=<START> <END>
                        Set the percentage bounds for custom charge. Both
                        must be integers. START must lie in the range [50,
                        95], END must lie in the range [55, 100], END must be
                        at least (START + 5)

[superm1]

Let me know if you're ok with this CLI:

I would prefer to see --set-custom-charge-interval split into two different CLI options that take an integer but otherwise it's fine.

[marmistrz]

I would prefer to see --set-custom-charge-interval split into two different CLI options that take an integer but otherwise it's fine.

Accepting the whole interval at once makes it much easier to validate its correctness.

[superm1]

Accepting the whole interval at once makes it much easier to validate its correctness.

Can't you just do something like this:

if options.set_custom_charge_start and options.set_custom_charge_end:
    set_custom_charge_interval  ((start,end))

And you just do your validation in that routine?

[marmistrz]

n't you just do something like this:

if options.set_custom_charge_start and options.set_custom_charge_end:
    set_custom_charge_interval  ((start,end))

And you just do your validation in that routine?

Then start & end have to be always supplied together, either none or both. Exposing them as separate options suggests otherwise. I know that the equality sign in the help is a little confusing, but I checked and argparse prints a much better help string (optparse is deprecated)

The PR should be ready.

[marmistrz]

When is the next release expected?

[superm1]

End of the summer was the plan