MichalW / gnome-bluetooth-battery-indicator

Gnome-Shell extension displaying battery percentage for bluetooth devices
GNU General Public License v3.0
121 stars 40 forks source link

Fixed case mismatch in UPower script #34

Closed mfontanaar closed 2 years ago

mfontanaar commented 2 years ago

I am not very experienced in UPower, however I have two devices whose battery level is reported through UPower but not through bluetoothctl.

However, in my installation (Fedora 35), and for the three BT devices I have upower --dump returns MAC addresses in lower case, whereas the extension always holds the MAC addresses in upper case. I consequently added a tr '[:lower:]' '[:upper:]' (conversion from lower to upper) after getting upower information before parsing devices with awk. This resolved the ? battery level in my case.

https://github.com/MichalW/gnome-bluetooth-battery-indicator/issues/31#issuecomment-1059751899 for pointing the issue.

MichalW commented 2 years ago

@eirik-ff Could you take a look?

eirikff commented 2 years ago

Looks good to me. :)

mfontanaar commented 2 years ago

Looks good to me. :)

Just out of curiosity, to have some closure. Does upower --dump returns the MAC in uppercase in your setup or does the extension holds it in lower case?

eirikff commented 2 years ago

Yes, for me it returns the MAC address in uppercase. I'm not sure if the extension holds the address in upper or lower case, I guess it depends on what is returned here: https://github.com/MichalW/gnome-bluetooth-battery-indicator/blob/master/bluetooth.js#L85 But in my case, that seems to be in uppercase.

mfontanaar commented 2 years ago

Exactly, for me it returns uppercase too. I came to that line but saw it was using the Gnome Instrospection API, which I completely lack knowledge of and for which I couldn't find information. Perhaps @MichalW knows if it is safe to assure that line will return an upper case address.

In that case, the variability is only in the UPower output, which for me (and some other people) is lowercase. I also couldn't find if it was a deliberate change in UPower or what exactly does case depends on, but with this MR it should be anecdotal anyway.