Don't call `Refresh` on UPower devices

[CommandMC]

As mentioned in #63, removing those two lines (to be compatible with the most recent UPower release) did not seem to affect program functionality, so I don't know why they were there

[joaorb64]

Thanks for the contribution!

[GuillaumeCisco]

I was digging around this issue since this morning. Why do we remove these lines? Didn't we use them for refreshing the status of the controllers?

During my researches, I came across this commit: https://gitlab.freedesktop.org/upower/upower/-/commit/d0ebbe32bb5cec06335a7bd0f11f8550deaec16e introduced in version 0.99.18: https://gitlab.freedesktop.org/upower/upower/-/releases/v0.99.18

Looks like there is another way to call the refresh method. I asked to @hadess : https://github.com/hadess on his twitter https://twitter.com/hadessuk Looks like he is the author of the commit. Maybe it can helps.

To put in a nutshell, the Refresh method is only available in debug mode now. And regarding the commit in upower. It looks like we should use another way to refresh the state of the controller.

Using debug mode from upower:

In addition, it can also be useful to run upower in debug mode and post the logs. There are two ways of doing so: \* Run upower daemon manually, you can do so using: \`sudo /usr/libexec/upowerd -rd\` \* Modify the systemd service and restart. This is best done by: 1. \`sudo systemctl edit upower.service\` 2. Adding the two lines: \`\`\` \[Service\] Environment=G_MESSAGES_DEBUG=all \`\`\` 3. \`sudo systemctl restart upower.service\` 4. Grab logs using \`journalctl -u upower.service\` or similar

First method worked in my case, but second one did not... Maybe you could test it too and tell me if it works on your end.

I'm still digging in this project I don't really know...

Cheers,

[CommandMC]

Upower should take care of refreshing this information for us, as far as I've understood From the commit you linked:

This is not needed as all the backends should be sending events when their states changes, removing the need for an explicit refresh.

This is also a potential security problem if applications keep on refreshing their data.

After removing those lines here, I was not able to notice any difference in functionality

[GuillaumeCisco]

@CommandMC Is battery level still updated? Or do we need to subscribe to an event from the device?

[CommandMC]

Last time I checked, battery level was reported & updated fine.
Other than that, I really don't know. I'm not familiar with Upower, I just ran into the issue of this no longer working, traced back the problem to those two lines, and removing them seemed to just "make it work"

[hadess]

Does someone want to explain what this is about? :)

The Refresh() wasn't needed for any of the backends that upower supports, all the backends (kernel, bluez, libimobiledevice) have their own way of updating the backend based on events on the system.

What does your code do, and why did you need to call Refresh()? It would just have forced re-reading battery states, draining the battery in the process.

If you want to check whether you receive events on battery changes run upower --monitor-detail, you should see battery information when the battery level changes (see the updated field for when it did).

[CommandMC]

What does your code do, and why did you need to call Refresh()?

This is also what I was puzzled about when making this PR. But again, I'm just an end user who ran into this problem, I'm in no way knowledgeable about how this project works in its entirety; sorry I'm not able to explain it in any more detail

[GuillaumeCisco]

Same as @CommandMC, I'm not the owner of this project. But from what I understand battery level was read every 30 seconds. In order to update it on the GUI:

    async def _get_battery_level(self):
        print_verbose("Battery level reading thread started")
        try:
            while self.dbus_interface != None:
                self.dbus_interface.Refresh()
                properties = self.dbus_properties_interface.GetAll("org.freedesktop.UPower.Device")
                print_verbose("Battery level update")
                if properties["Percentage"] != self.battery:
                    print_verbose("Battery level changed")
                    self.battery = properties["Percentage"]
                    self.server.print_slots()
                await asyncio.sleep(30)
        except (asyncio.CancelledError, Exception) as e:
            print_verbose("Battery level reading task ended")
            self.battery = None

Now I wonder if removing the Refresh part still allow us to display the correct percentage of battery. Also from my tests, nintendo switch controller pro only display 100,70,55,30,0 battery percentages... So it is hard to test...

Is there any way to subscribe to battery percentage events from python? Using dbus and upower? I'm new to these two tools...

By the way, thank you very much @hadess for answering to this topic.

[hadess]

Reading the code, it's just trying to get the latest battery percentage, and now it will just get the latest percentage that was read.

This part of the code could probably be rewritten to use the fact that D-Bus sends signals when the properties changes, rather than polling upower every 30 seconds to get information that might be at most 30 seconds old. But that would require integrating it into a mainloop, and I didn't read enough of the code to see what exists there.

In any case, this change is fine, and it would be nice to clean up that code, but not a major problem.

[GuillaumeCisco]

I wrote a little script allowing me to see all the signals on dbus:

import dbus
from dbus.mainloop.glib import DBusGMainLoop
from gi.repository import GLib

def handle(*args):
    print(args)

def main():
    dbus_loop = DBusGMainLoop()
    bus = dbus.SystemBus(mainloop=dbus_loop)  # connect to system wide dbus
    bus.add_signal_receiver(  # define the signal to listen to
        handle,  # callback function
        None,  # signal name
        None,  # interface
        None  # bus name
    )

    loop = GLib.MainLoop()
    loop.run()

if __name__ == "__main__":
    main()

I can also see on dbus upower documentation We can subscribe to two signals:

• DeviceAdded
• DeviceRemoved

Not really what we wants...

But on the Device documentation, there are no signals.

Looks like we are very close to a solution. But I cannot figure out how to subscribe to the signal for battery update on the nintendo switch controller. I tested the add/remove via bluetooth signals and they work well.

Maybe it is not possible to know the signals without nintendo documentation? Or maybe it is standardized?

Any thoughts @hadess, I think we are very close :)

[hadess]

I'm sorry, but I'm not going to have time to explain how D-Bus works but what you're looking for is the PropertiesChanged() signal on the standard org.freedesktop.DBus.Properties interface.

$ gdbus introspect --system --dest org.freedesktop.UPower --object-path /org/freedesktop/UPower/devices/battery_hidpp_battery_0
[...]
  interface org.freedesktop.DBus.Properties {
    methods:
      Get(in  s interface_name,
          in  s property_name,
          out v value);
      GetAll(in  s interface_name,
             out a{sv} properties);
      Set(in  s interface_name,
          in  s property_name,
          in  v value);
    signals:
      PropertiesChanged(s interface_name,
                        a{sv} changed_properties,
                        as invalidated_properties);
    properties:
  };
[...]
  interface org.freedesktop.UPower.Device {
[...]
    properties:
[...]
      readonly t UpdateTime = 1666964619;
[...]
      readonly d Percentage = 100.0;
[...]
  };
};

Try gdbus monitor ... to see where the signals come from and what they contain. You'll find plenty of examples online on stackoverflow and other free software programs.

[GuillaumeCisco]

Thanks a lot for all this help @hadess. I'm finally close to a solution. I have something that works. I'm able to subscribe to the device thanks to:

   bus.add_signal_receiver(handler, signal_name='PropertiesChanged', bus_name=dev.bus_name, path=dev.object_path)

When I call Refresh every two seconds on it, I can see the property UpdateTime which is updated. Only need to bind the signal to Percentage now :) And remove the Refresh+polling code.

I was doing a mistake with signals and properties... Inverting them...

I read a lot about dbus today, and understood a lot of things. Thanks a lot for all the help.

I will come back with a solution very soon :)

[GuillaumeCisco]

I made a PR with updates: https://github.com/joaorb64/joycond-cemuhook/pull/69

Could you please review it @CommandMC? I'd like your advices. A new dependency needs to be installed: PyGObject-stubs

Thanks a lot!

[joaorb64]

Great discussion over here guys. Just wanted to comment that I'm around but I don't know a lot about dbus/upower myself, it was just what I could put up to get the battery level at the time. I really appreciate the help/discussion/PR! I'll also take a look at it and wait for CommandMC's review for the merge!