maniacx / Battery-Health-Charging

GNU General Public License v3.0
157 stars 14 forks source link

setting show-dell-option accessed in lib/notifier.js but not defined in schema #77

Closed prahal closed 1 month ago

prahal commented 4 months ago

I get this error:

gnome-shell[6286]: JS ERROR: Error: GSettings key show-dell-option not found in schema org.gnome.shell.extensions.Battery-Health-Charging
                                             _checkKey@resource:///org/gnome/gjs/modules/core/overrides/Gio.js:779:23
                                             createCheckedMethod/<@resource:///org/gnome/gjs/modules/core/overrides/Gio.js:729:30
                                             notifyThresholdNotUpdated@file:///home/prahal/.local/share/gnome-shell/extensions/Battery-Health-Charging@maniacx.github.com/lib/notifier.js:90:28
                                             ChargeLimitToggle/<@file:///home/prahal/.local/share/gnome-shell/extensions/Battery-Health-Charging@maniacx.github.com/lib/thresholdPanel.js:287:36
                                             _reVerifyThreshold@file:///home/prahal/.local/share/gnome-shell/extensions/Battery-Health-Charging@maniacx.github.com/devices/Thinkpad.js:308:14
                                             setThresholdLimit/this._delayReadTimeoutId<@file:///home/prahal/.local/share/gnome-shell/extensions/Battery-Health-Charging@maniacx.github.com/devices/Thinkpad.js:28>
                                             @resource:///org/gnome/shell/ui/init.js:21:20

I might try to cook a PR but I am a bit rusty with my glib schema fu. So please beat me to it.

I am running version 51 of the extension on gnome-shell 45.3 on Debian testing.

Edit: Mind it seems these calls are a leftover, so no need to add them to the schema. @maniacx in commit 85091248583ab0f9996b9a04fd6dce861ff3b86a "Battery Health Charging: Implement cctk bios password validation for dell" you removed most show-dell-option accesses except the two in lib/notifier.js. "'show-dell-option" was the value of what is now in both "detected-libsmbios" and "detected-cctk", but maybe the notifier messages for notifyAnErrorOccured and NotifyThresholdNotUpdated that are to be notified when show-dell-option was set should be reworked too.

maniacx commented 4 months ago

Thank you for reporting this and pointing out the solution as well Yes these are left over code, a mistake made by me. This should fix the error and it will display the notification.

But now the bigger question is why did notifyThresholdNotUpdated get triggered in the first place.

'/sys/class/power_supply/BAT0/charge_control_end_threshold';
 '/sys/class/power_supply/BAT0/charge_control_start_threshold';

Looking at the logs seems like you are using a thinkpad. After applying the threshold the extension verifies the threshold by reading the values to check if the threshold it updated. If not updated , it reads and checks again after 200ms (as some device doesnt update immediatly). But seem like you threshold is not getting update even after 200ms.

Let me know what is the results

prahal commented 4 months ago

The error was somewhat expected because the kernel support for this Lenovo ThinkPad Yoga S1 is left to be desired (well it was unstable for a very long time for Windows users too, maybe 6 years). It is from 2014...

cat /sys/class/power_supply/BAT0/charge_control_end_threshold
208
cat '/sys/class/power_supply/BAT0/charge_control_start_threshold'
208

Setting the threshold via tlp-bat was working even though the values read were not correct. It seems setting the thresholds under Battery Health does not work ... I might as well disable it until I find a way or someone that can fix the sysfs entries for this ThinkPad Yoga S1.

Also the "force-discharge" in BAT0/charge_behavior hangs the board (though I believe I had it working once a few years ago, though I am not sure (I was using the tlp wrapper). The cycle count works under Windows but not Linux sysfs.

The Battery Health Quick Setting button when expanded shows the thresholds as Nan/NaN.

What might be great is to have a "full charge once" button to use just before a trip. But that seems out of scope for a Battery Health tool.

Edit: well turned out the Battery Health properly changed the threshold but told me it failed when I switched to "Full Capacity". Probably because the sysfs entries values are wrong. If I disable the extension and reenable it the extension properly shows it as set to "Full Capacity" (when I switched it to "Full Capacity" from "Balanced" the Button text stayed to "Balanced" (probably because of the error reading the threshold values).

prahal commented 4 months ago

About the fix to test. I made the same change localy but was not confident it was fine. It fixes the message from gnome-shell and I get the error notification (translated):

Battery Health Charging Error
Stop threshold (Thinkpad BAT0)
maniacx commented 4 months ago

If the sysfs displays incorrect value when read, then the extension will not work. There are couple of things you can try though

  1. Turn off and Reset EC values. (Note this will factory reset your bios settings so make a note of any changes, before doing this)
  2. Use the latest BIOS firmware update.

Also note sometime the latest BIOS firmware will not show on linux fwupd, so you might have to boot to windows to find if there is a firmware update for your model using oem software.

Also I do not want to be responsible for any damages due to EC reset or firmware update. if you decide to proceed read OEM's instruction carefully and proceed.

You can read this dicussion of another user using linux (but not using this extension) having the same issue, yoga x380 reads 208, the user solved the issue by updating to the latest firmware in Windows. Read the whole post for better undestanding..

https://www.reddit.com/r/thinkpad/comments/175m0l6/comment/k5cfcfp/

Let me know if you decide to proceed and try steps mentioned above.


Screenshot from 2024-02-06 16-45-01


Screenshot from 2024-02-06 16-17-45

prahal commented 4 months ago

I already installed the latest BIOS but it is very old (no update since 30 April 2020, but this is an old laptop). But it is far from perfect. It DSDT even defines EC thermal to be at offset 0x668 when the ACPI spec does not support that and Embedded Controler max control offset is 0xFF... (obviously Linux complains when it parses the table that it is invalid...). How such an obvious error could have ended up in a release and even not fixed from 2013 to 2020... I mean this looks like do not touch the code if it works (well I believe if it does not freeze in a way you cannot hide).

I read the link you provided. The emergency reset button did not help, only the BIOS update did. Still, I wonder how the Lenovo vantage application on Windows manages to get correct values (and force discharge the battery as it works with this tool under Windows). It also shows the cycle count of the battery.

I won't be able to do it soon, but do you want me to make the fix you gave me above into a PR? (I believe from this patch that removing the show-dell-option special handling for the error messages is the way to go)

maniacx commented 4 months ago

I already create a PR and submitted for release, for that patch. Thank for the help.

Since reading for sysfs is buggy. I can change the extension behaviour just to apply the threshold without reverfying if it is applied or not. But I cannot add this feature to the extension as this may effect other users.

Try this. Battery-Health-Charging-disable-verification.zip

I have change removed the reverification part (Only for Thinkpad BAT0). Now the extension will just apply the threshold. I also changed the extensions UUID so uninstall the original Battery Health Charging extension before installing this one. Since the UUID is different, it will never update as extension/extension manager app will think it is a differnet extension

Original UUID Battery-Health-Charging@maniacx.github.com Above extension UUID Battery-Health-Charging@prahal.github.com

I havent even tested it though so I dont know if i have missed something.