Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add UPower Service #10

Merged
merged 21 commits into from
Oct 13, 2024
Merged

feat: add UPower Service #10

merged 21 commits into from
Oct 13, 2024

Conversation

linkfrg
Copy link
Owner

@linkfrg linkfrg commented Oct 3, 2024

Add battery UPower Service.
Completely stolen from AGS (Now it's 100% Ignis implementation)
I don't have a device with a battery, so this pull request requires testing by other people.

  • Testing
  • Documentation
  • Charge Threshold support
  • Multiple batteries support
  • Move to UPowerGlib

How to install:

  1. Set up a development environment
  2. git checkout feat/battery.
    On each new commit use git pull to receive new changes

@Munseer-am
Copy link

Add battery service. Completely stolen from AGS I don't have a device with a battery, so this pull request requires testing by other people.

  • Testing

I will help out with testing when I can!

@linkfrg
Copy link
Owner Author

linkfrg commented Oct 3, 2024

this terrible code can help you with testing:

from ignis.services.battery import BatteryService

battery = BatteryService.get_default()

for prop_name in [
    "available",
    "percent",
    "charging",
    "charged",
    "icon_name",
    "time_remaining",
    "energy",
    "energy_full",
    "energy_rate",
]:
    battery.connect(
        f"notify::{prop_name.replace('_', '-')}",
        lambda x, y: print(f"{prop_name}: ", getattr(battery, prop_name)),
    )

@Munseer-am
Copy link

Issue: I am encountering an error when running the ignis init --debug.

ignis.exceptions.AnotherNotificationDaemonRunningError: Another notification daemon is already running: SwayNotificationCenter

@linkfrg
Copy link
Owner Author

linkfrg commented Oct 3, 2024

Issue: I am encountering an error when running the ignis init --debug.

ignis.exceptions.AnotherNotificationDaemonRunningError: Another notification daemon is already running: SwayNotificationCenter

This is not an issue, this exception means that you have another notification daemon running. In your case it is swaync, kill or remove it

Please read the error text carefully

@Munseer-am
Copy link

from ignis.services.battery import BatteryService

battery = BatteryService.get_default()

print(battery.available)

it is returning False

@linkfrg
Copy link
Owner Author

linkfrg commented Oct 3, 2024

try now (after 87ea7d9)

@linkfrg linkfrg added the enhancement New feature or request label Oct 3, 2024
@linkfrg linkfrg self-assigned this Oct 3, 2024
@CGod-dev
Copy link

CGod-dev commented Oct 3, 2024

I don't have a device with a battery, so this pull request requires testing by other people.

sudo udevadm trigger --subsystem-match=power_supply --action=add

@linkfrg
Copy link
Owner Author

linkfrg commented Oct 3, 2024

sudo udevadm trigger --subsystem-match=power_supply --action=add

nothing changed

@Munseer-am
Copy link

Munseer-am commented Oct 3, 2024

try now (after 87ea7d9)

it is still returning False and it has a new error

battery = BatteryService.get_default()
Error while fetching Present property: 'DBusProxy' object has no attribute 'Present'
None

@CGod-dev
Copy link

CGod-dev commented Oct 3, 2024

sudo udevadm trigger --subsystem-match=power_supply --action=add

nothing changed

You can "simulate" a battery by using symlink

mkdir BAT0
sudo ln -s BAT0/ /sys/class/power_supply/BAT0/
echo 100 | sudo tee BAT0/capacity
echo "Charging" | sudo tee BAT0/status
echo 4200000 | sudo tee BAT0/voltage_now
echo 5000000 | sudo tee BAT0/current_now

Although, this method is ancient. I don't know if it will work on modern linux

@linkfrg
Copy link
Owner Author

linkfrg commented Oct 4, 2024

try now (after 87ea7d9)

it is still returning False and it has a new error

battery = BatteryService.get_default()
Error while fetching Present property: 'DBusProxy' object has no attribute 'Present'
None

Have you installed upower?

@CGod-dev
Copy link

CGod-dev commented Oct 4, 2024

Being able to set a charge limit from the quick settings would be nice too, somewhat like this gnome extension

@linkfrg
Copy link
Owner Author

linkfrg commented Oct 4, 2024

sudo udevadm trigger --subsystem-match=power_supply --action=add

nothing changed

You can "simulate" a battery by using symlink

mkdir BAT0
sudo ln -s BAT0/ /sys/class/power_supply/BAT0/
echo 100 | sudo tee BAT0/capacity
echo "Charging" | sudo tee BAT0/status
echo 4200000 | sudo tee BAT0/voltage_now
echo 5000000 | sudo tee BAT0/current_now

Although, this method is ancient. I don't know if it will work on modern linux

It seems that /sys cannot be modified. But I found some kernel module and it's working

@CGod-dev
Copy link

CGod-dev commented Oct 4, 2024

this terrible code can help you with testing:

from ignis.services.battery import BatteryService

battery = BatteryService.get_default()

for prop_name in [
    "available",
    "percent",
    "charging",
    "charged",
    "icon_name",
    "time_remaining",
    "energy",
    "energy_full",
    "energy_rate",
]:
    battery.connect(
        f"notify::{prop_name.replace('_', '-')}",
        lambda x, y: print(f"{prop_name}: ", getattr(battery, prop_name)),
    )

Finally got the pull request working. Here's the output (Battery is at 98 and AC is plugged in)

2024-10-04 19:03:15 [INFO] Using configuration file: /home/matt/.config/ignis/config.py
2024-10-04 19:03:15 [INFO] Applied css: /home/matt/.config/ignis/style.scss
2024-10-04 19:03:15 [INFO] Ready.
energy_rate:  0.0

@linkfrg
Copy link
Owner Author

linkfrg commented Oct 4, 2024

D-Bus proxy could not update properties because it was looking for the name in the current session bus, not in the system one. Should be fixed after 4f09d4a

Edit: strange but available is still False
Edit 2: This is because self._available never set to True in self.__sync d426567

@dylanc24
Copy link

dylanc24 commented Oct 4, 2024

If I was smarter, I would test before It get push... But here I am, too stupid to uninstall and rebuild the package, hoping it come soon. .

@CGod-dev
Copy link

CGod-dev commented Oct 4, 2024

D-Bus proxy could not update properties because it was looking for the name in the current session bus, not in the system one. Should be fixed after 4f09d4a

Edit: strange but available is still False Edit 2: This is because self._available never set to True in self.__sync d426567

nergy_rate:  7.404
energy_rate:  7.404
energy_rate:  7.404
energy_rate:  7.404
energy_rate:  7.404
energy_rate:  7.404
energy_rate:  10.605
energy_rate:  10.605
energy_rate:  10.605
energy_rate:  10.605
energy_rate:  10.605
energy_rate:  10.605
energy_rate:  10.605
energy_rate:  10.605
energy_rate:  10.605

Looks to be working now. Can you provide instructions for fast rebuilding and installing? I am not very familiar with meson or python

@linkfrg
Copy link
Owner Author

linkfrg commented Oct 4, 2024

If I was smarter, I would test before It get push... But here I am, too stupid to uninstall and rebuild the package, hoping it come soon. .

You can Set up a development environment and when:
git checkout feat/battery. On each new commit use git pull to receive new changes

@linkfrg
Copy link
Owner Author

linkfrg commented Oct 4, 2024

Can you provide instructions for fast rebuilding and installing? I am not very familiar with meson or python

Added instruction in the first message

@CGod-dev
Copy link

CGod-dev commented Oct 4, 2024

Can you provide instructions for fast rebuilding and installing? I am not very familiar with meson or python

Added instruction in the first message

I know how to use git. Just can't get meson to see the changes

meson compile -C build
INFO: autodetecting backend as ninja
INFO: calculating backend command to run: /sbin/ninja -C /redacted/for/privacy/ignis/build
ninja: Entering directory `/redacted/for/privacy/ignis/build'
ninja: no work to do.

@linkfrg
Copy link
Owner Author

linkfrg commented Oct 4, 2024

Can you provide instructions for fast rebuilding and installing? I am not very familiar with meson or python

Added instruction in the first message

I know how to use git. Just can't get meson to see the changes

meson compile -C build
INFO: autodetecting backend as ninja
INFO: calculating backend command to run: /sbin/ninja -C /redacted/for/privacy/ignis/build
ninja: Entering directory `/redacted/for/privacy/ignis/build'
ninja: no work to do.

You don't need to rebuild Ignis every time. You need to do this in the first time to build submodules and install bin file. Since we are making a symbolic link in the above tutorial, we don't need to reinstall Ignis every time to update it in venv.

@linkfrg
Copy link
Owner Author

linkfrg commented Oct 4, 2024

Being able to set a charge limit from the quick settings would be nice too, somewhat like this gnome extension

added to tasks

@linkfrg
Copy link
Owner Author

linkfrg commented Oct 4, 2024

Issue: The service seems to get confused when there is 2 batteries, see below:

energy_rate:  10.204
energy_rate:  0
energy_rate:  10.204
energy_rate:  0
energy_rate:  10.204
energy_rate:  0
energy_rate:  10.204
energy_rate:  0
energy_rate:  10.204
energy_rate:  0

Edit: Thinkpads have 2 batteries, one being internal and one being external; /sys/class/power_supply/BAT0 and /sys/class/power_supply/BAT1 in order.

Only the energy_rate values ​​differ?

From upower docs:

Amount of energy being drained from the source, measured in W. If positive, the source is being discharged, if negative it's being charged.

Maybe the power consumption is just not constant, so it drops to zero and rises back to 10.204

@linkfrg linkfrg marked this pull request as draft October 5, 2024 04:24
@Munseer-am
Copy link

Munseer-am commented Oct 5, 2024

The battery percentage displays correctly, and I have implemented the battery in the bar, but battery.charged and battery.charging don't seem to work properly; both return False, even though the upower command shows that the battery state is charging.

@linkfrg
Copy link
Owner Author

linkfrg commented Oct 6, 2024

9caa6b4: Moved to UPowerGlib, also added support for managing multiple batteries, need testing.

There are two properties for BatteryService: devices and display_device.

devices (list[Battery]): list of all batteries
display_device (Battery): The battery to display (main battery?)

I will not describe all the properties for Battery, you can look at them directly in the code

@artinmare
Copy link

Hi, sorry I'm new to Linux and git in general so please pardon my clumsiness

First of all, love the work, I have been testing this and it seems to work fine for me. My laptop only have a single battery so cannot test for the multiple one, but everything else seems to be good

The only problem i found is the time_remaining prop which failed cause there is no self._charging attribute in Battery class. It seems to be a typo, I got it working by changing the self._charging to self.charging in the time_remaining function line 96 in the battery.py file.

Didn't know how to make a pull request so that's about it. I would report if I found another issues, love my experience with this software so far, thank you for creating it.

@linkfrg
Copy link
Owner Author

linkfrg commented Oct 7, 2024

The only problem i found is the time_remaining prop which failed cause there is no self._charging attribute in Battery class. It seems to be a typo, I got it working by changing the self._charging to self.charging in the time_remaining function line 96 in the battery.py file.

fixed in faf4e41

@linkfrg
Copy link
Owner Author

linkfrg commented Oct 12, 2024

56ac88d
BREAKING CHANGE: BatteryService renamed to UPowerService.

Now this service has support for non battery devices.

In particular, batteries have been moved to the new property: batteries.

@linkfrg linkfrg changed the title feat: add Battery Service feat: add UPower Service Oct 12, 2024
@linkfrg
Copy link
Owner Author

linkfrg commented Oct 12, 2024

The only thing left is to test charge threshold

To check if your battery supports the charge threshold, use the UPowerDevice.charge_threshold_supported.

If True, enable it by setting charge_threshold property to True and try setting charge_start_threshold and charge_end_threshold

@linkfrg
Copy link
Owner Author

linkfrg commented Oct 13, 2024

no activity :(

charge threshold support will be removed

@linkfrg linkfrg marked this pull request as ready for review October 13, 2024 18:00
@linkfrg linkfrg merged commit 4246898 into main Oct 13, 2024
4 checks passed
@linkfrg linkfrg deleted the feat/battery branch October 13, 2024 18:07
@prakharn prakharn mentioned this pull request Oct 20, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants