lxde / lxpanel

Other
56 stars 42 forks source link

Handle charge_full and energy_full not available #2

Closed sdgathman closed 3 years ago

sdgathman commented 5 years ago

This just handles a specific device, pine64. What really needs to happen is to make the charge_full a configured item for devices that don't supply the information. The battery driver supplies percentage of charge remaining in capacity, but no charge_full or energy_full.

sdgathman commented 5 years ago

I tested and found the battery window entirely accurate with this patch on my pine64. Of course, since charge_full is hardwired to 10000, it has limited utility on other devices triggering the condition.

sdgathman commented 5 years ago

Is this really a bug in the battery driver? Maybe charge_full is something that should be provided in the device tree for ARM devices?

Otherwise, I can manually edit ~/.config/lxpanel/LXDE/panels and add ChargeFull to the batt plugin and code to batt.c to read it. Someone else can add it to the gui - although if only a few devices are like this, it might be clutter in the gui. The config_settings seem to be for the GUI only, however. It seems like mixing levels to add ChargeFull there.

LStranger commented 3 years ago

Looks fine. Let hope it will not break other laptops indication. Thank you very much. Going to merge it.

sdgathman commented 3 years ago

I put the battery status logic in a python cli script, which I run on half a dozen devices to ensure it works on them all (all with slightly different data available from hardware driver). Does it seem worthwhile to try to update lxpanel to support the additional devices? I only run LXDE on one device currently (a Pinebook with only 2G ram).

On other devices, I just run the script in a panel applet that displays the output of a command.

https://github.com/sdgathman/batt

sdgathman commented 3 years ago

So with the additional testing coverage from the python script above, hardwiring 10000mAh is not a good idea. We need to provide a config entry. For the time being, it can be just for that value. I did discover that some models do not provide charge_full_design, but do provide energy_full_design. (And calculating one from the other requires some voltage data, hence the need for voltage_max and voltage_min.)

LStranger commented 3 years ago

So with the additional testing coverage from the python script above, hardwiring 10000mAh is not a good idea. We need to provide a config entry. For the time being, it can be just for that value. I did discover that some models do not provide charge_full_design, but do provide energy_full_design.

Well, at least it shouldn't be worse than before, I hope. If you come with any better improvement, you're always welcome.

sdgathman commented 3 years ago

Fun. HP Elitebook does not provide current_now. You have to keep a history of charge_now and estimate it yourself. So the applet is going to have to have state. Possibly in memory is good enough.