BP thresholds don't make sense: min 0%, max 0% about matebook-applet HOT 8 CLOSED

Are you able to change the battery settings, though?

from matebook-applet.

Last night, no - the GUI showed the error message and the Fn-Lock status, but oddly enough, it was working this morning and now, even though I had restarted and reinstalled several times after installing. Not sure what caused it, but in any case it's not a problem anymore. Thanks for the hard work!

from matebook-applet.

Hmmm… I have a feeling there's something going on with 2020 Matebooks, that is, they're a little bit different from the older tried-and-tested models and may even need some changes in the driver (confer #22 for instance), but not that much different so as to provoke a thorough investigation.

In any case, here's what's been happening to you: the driver reported the battery thresholds to be set to 0 0 (that is, 0% for the low and 0% for the high threshold). which isn't supposed to happen on pre-2020 models, where 0 100 is "battery protection off", and 0 0 is never seen in the wild. I think (although I don't remember that clearly) that one of the earliest versions of the driver would report thresholds as 0 0 when it couldn't get them, which is why matebook-applet thinks something is wrong when it gets 0 0 from the driver. The most confusing thing is that the issue seems to fix itself after a while, so there seems to be no way to properly investigate (other than purchasing a 2020 Matebook and sending it to @aymanbagabas as a gift).

Don't get me wrong, I'm willing to fix this — if, indeed, this is the applet's fault. As I see it, one of the following is true:
either

1. 0 0 is a valid reply for the 2020 Matebooks, in which case matebook-applet needs to be fixed so that when it sees 0 0, it interprets it as "protection off", not as "something is wrong",
   or
2. the driver produces 0 0 on 2020 Matebooks in some cases because something is wrong with the driver, in which case matebook-applet is correct in interpreting it as "something is wrong", and it's the driver that needs to be fixed.

Unfortunately, I don't know squat about the inner workings of the driver, and I wouldn't be able to see the problem with it (or lack thereof) even if I had the relevant lines of code highlighted in front of me, so I'm waiting for someone with authority and knowledge to say whether matebook-applet needs to be fixed here or not.

Sorry for the inconvenience.

from matebook-applet.

It's definitely interesting that it just "fixes itself" - I can anecdotally say that it seemed to be working after using it on battery power for a while - something I hadn't done much. In any case, it's fixed and is working fine, no worries :)

from matebook-applet.

@nekr0z the problem with #22 is strange, it returns operation not permitted for no known reason as you can see when comparing the logs. We really need to examine that one further.

Apart from that, the driver makes no distinction of the validity of the returned value. The way I'm handling setting the value is

if (start < 0 || end < 0 || start > 100 || end > 100)
		return -EINVAL;

So reversing this logic, the returned value is valid iff start <= 100 && end <=100 && start >= 0 && end >= 0. My Matebook X Pro (MACH-WX9) sometime returns 0 0 after multiple reboots without setting battery protection.

Looking at the implementation of the set/get functions in some of these devices, each implementation is different in a way, some devices do validation and some don't. When i wrote this, my intent was to redirect any unknown behavior back to user-space to keep it simple and compatible with most devices.

With that being said, in the gui, I'd treat reading both 0 0 and 0 100 as a valid value for battery protection off but always use 0 100 when setting it.

from matebook-applet.

@aymanbagabas Thanks for the insight, man! I think I'll get rid of that check in the applet.

I'd treat reading both 0 0 and 0 100 as a valid value for battery protection off but always use 0 100 when setting it

Yep, a totally reasonable approach, I'll adapt it in the next version of the applet. I'm not sure about one case, though: at startup, the applet tries to figure out whether it has write permissions on the driver endpoint, and it does that by attempting to write the same value it read. When it reads 0 0, it currently attempts to write 0 0, which is exactly how #22 was discovered.

I'm reluctant to change the logic for this check, because at that point the applet isn't really supposed to make any changes to the settings, not even fix 0 0 to 0 100...

from matebook-applet.

@nekr0z you shouldn't really write to the endpoint to check for write permissions. The endpoint should always be writable as defined in the driver.

static DEVICE_ATTR_RW(charge_control_start_threshold);
static DEVICE_ATTR_RW(charge_control_end_threshold);
static DEVICE_ATTR_RW(charge_control_thresholds);

As long as you have root, you'd be able to write to the endpoint. You could also check the file permissions to verify that you can write to it.

from matebook-applet.

@aymanbagabas Maybe you're right... Trying to actually write serves as a quick (and, admittedly, dirty) way to check permissions, existence and whatever else can get in the way in one go, but maybe I should find some time and make the applet be more meticulous and do the actual checks.

from matebook-applet.