Merge ~ghadi-rahme/ubuntu/+source/powermgmt-base:ubuntu/devel into ubuntu/+source/powermgmt-base:ubuntu/devel

[Ghadi Rahme]

on_ac_power: Fix incorrect AC state on some machines with USB type-C ports (LP: #1980991)

On some machines, on_ac_power will incorrectly report the power state due to USB-C ports.

The current implementation of on_ac_power only checks if the USB-C port is in the connected state without checking first if any of the ports are running in sink or source mode.
With this patch, on_ac_power will first make sure that there is at least a USB port operating in sink mode before considering the USB* type as a possible power supply source.
If no source mode USB-C devices are found assume that there are USB-C ports in sink mode.

This implementation mimics what is already present in systemd with the systemd-ac-power command, where if a USB-C port in source mode is found, USB-C ports are ignored when determining the current power state.

The source code for the systemd implementation can be found here: https://github.com/systemd/systemd/blob/main/src/shared/battery-util.c#L12

[Simon Chopin (schopin)]

Hi Ghadi,

Thanks a lot for taking this on.

It looks pretty solid, but I do have some questions and minor issues.
In addition to the diff comments, would you be OK splitting the changes to debian/changelog into its own commit? It makes it easier to maintain the delta further down the line when using git-ubuntu, see https://github.com/canonical/ubuntu-maintainers-handbook/blob/main/PackageMerging.md

Cheers,
Simon

review: Needs Fixing

[Ghadi Rahme (ghadi-rahme)]

Thank you for the review Simon!

I will answer your question, follow your suggestions, fix the issues you pointed out and split the commits before uploading a new version of the patch.

Regards,
Ghadi

[Simon Chopin (schopin)]

Please request a new review from ubuntu-sponsors once you think you've addressed all my comments :)

[Ghadi Rahme (ghadi-rahme)]

> Please request a new review from ubuntu-sponsors once you think you've
> addressed all my comments :)

Sounds good, I will request a review now just to get some feedback on how to move forward with the line you had a question about. In case I should remove it or keep it. That way I can do all the changes at once.

[Ghadi Rahme (ghadi-rahme)]

I went ahead and addresses the changes requested, I also removed the extra line I had added to get around a kernel bug, I think this should be fixed in the kernel and not have a workaround in this package.
I also split the commits in two with the changelog being the top most.

Let me know how it looks!

[Dan Bungert (dbungert)]

Hi Ghadi, a few more suggestions, see inline comments.

Also, I suggest forwarding this to Debian - I believe that https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1025141 is the same issue as discussed here, forwarding a debdiff to that bug would be nice. Thank you.

[Ghadi Rahme (ghadi-rahme)]

Hello Dan, thank you for the feedback! I will forward it to debian as you recommended once I have the patch ready.

057e9d3... by Ghadi Rahme on 2024-07-30

on_ac_power: Fix incorrect AC state on some machines with USB type-C ports

35149b8... by Ghadi Rahme on 2024-07-30

d/changelog: Add changelog entry for LP: #1980991 fix

[Ghadi Rahme (ghadi-rahme)]

Hello Dan,

I have applied the changes requested and edited the file using vim with the config that Simon recommended. Let me know how it looks!

Unmerged commits

35149b8... by Ghadi Rahme on 2024-07-30

d/changelog: Add changelog entry for LP: #1980991 fix

057e9d3... by Ghadi Rahme on 2024-07-30

on_ac_power: Fix incorrect AC state on some machines with USB type-C ports

diff --git a/debian/changelog b/debian/changelog
index 9394fc6..0d0c179 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+powermgmt-base (1.37+nmu2) oracular; urgency=medium
+
+  * Fix on_ac_power incorrectly reporting AC state on some
+    machines with USB type-C ports (LP: #1980991)
+
+ -- Ghadi Elie Rahme <[e-mail]>  Tue, 30 Jul 2024 16:59:34 +0300
+
 powermgmt-base (1.37+nmu1) unstable; urgency=medium
 
   * Non-maintainer upload.
diff --git a/on_ac_power b/on_ac_power
index 963e914..2e65fc0 100755
--- a/on_ac_power
+++ b/on_ac_power
@@ -21,13 +21,41 @@ set -e
 # no adapters are on-line but one or more are off-line, we return 1.
 #
 OFF_LINE_P=no
+USB_IS_SINK=no
+USB_IS_SOURCE=no
+power_type="Mains"
+
+# USB-C sysfs documentation:
+# https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-class-typec
+#
+# We verify first if there are any USB-C devices in power sink mode to
+# check if we should consider USB as a power source.  If no source ports
+# are found assume they are sink ports.
+#
+if [ -d /sys/class/typec/ ]; then
+	for FN in /sys/class/typec/*; do
+		if test -d "${FN}" && test -r "${FN}/power_role"; then
+			power_role="$(cat "${FN}/power_role")"
+			if echo $power_role | grep -q '\[source\]'; then
+				USB_IS_SOURCE=yes
+			elif echo $power_role | grep -q '\[sink\]'; then
+				USB_IS_SINK=yes
+			fi
+		fi
+	done
+	# Add USB* power type if any USB ports are sink or no source is found
+	if [ "${USB_IS_SINK}" = "yes" ] || [ "${USB_IS_SOURCE}" = "no" ]; then
+		power_type="USB*"
+	fi
+fi
+
 
 if [ -d /sys/class/power_supply/ ]; then
     for FN in /sys/class/power_supply/*; do
 	if test -d "${FN}" && test -r "${FN}/type"; then
 	    type="$(cat "${FN}/type")"
 	    case "${type}" in
-	    Mains|USB*|BrickID|Wireless)
+	    Mains|$power_type|BrickID|Wireless)
 		if [ -r "${FN}/online" ]; then
 		    online="$(cat "${FN}/online")"
 		    [ "$online" = 1 ] && exit 0