[PATCH v3 0/1] MR134: ntdll: Don't hard-code the battery and AC adapter names on Linux
Look through all of the devices in /sys/class/power_supply and take the statistics from the first battery and the first AC adapter. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52831 Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> -- v3: ntdll: Don't hard-code the battery and AC adapter names on Linux https://gitlab.winehq.org/wine/wine/-/merge_requests/134
From: Alex Henrie <alexhenrie24(a)gmail.com> Look through all of the devices in /sys/class/power_supply and take the statistics from the first battery and the first AC adapter. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52831 Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> --- dlls/ntdll/unix/system.c | 79 +++++++++++++++++++++++++++------------- 1 file changed, 54 insertions(+), 25 deletions(-) diff --git a/dlls/ntdll/unix/system.c b/dlls/ntdll/unix/system.c index 87cc8b9c3a4..1362143e29d 100644 --- a/dlls/ntdll/unix/system.c +++ b/dlls/ntdll/unix/system.c @@ -35,6 +35,7 @@ #include <errno.h> #include <sys/time.h> #include <time.h> +#include <dirent.h> #ifdef HAVE_SYS_PARAM_H # include <sys/param.h> #endif @@ -3379,12 +3380,14 @@ static ULONG mhz_from_cpuinfo(void) return cmz; } -static const char * get_sys_str(const char *path, char *s) +static const char * get_sys_str(const char *dirname, const char *basename, char *s) { - FILE *f = fopen(path, "r"); + char path[64]; + FILE *f; const char *ret = NULL; - if (f) + if (snprintf(path, sizeof(path), "%s/%s", dirname, basename) < 0) return NULL; + if ((f = fopen(path, "r"))) { if (fgets(s, 16, f)) ret = s; fclose(f); @@ -3392,42 +3395,68 @@ static const char * get_sys_str(const char *path, char *s) return ret; } -static int get_sys_int(const char *path, int def) +static int get_sys_int(const char *dirname, const char *basename) { char s[16]; - return get_sys_str(path, s) ? atoi(s) : def; + return get_sys_str(dirname, basename, s) ? atoi(s) : 0; } static NTSTATUS fill_battery_state( SYSTEM_BATTERY_STATE *bs ) { + DIR *d = opendir("/sys/class/power_supply"); + struct dirent *de; char s[16], path[64]; - unsigned int i = 0; + BOOL found_ac = FALSE; LONG64 voltage; /* microvolts */ - bs->AcOnLine = get_sys_int("/sys/class/power_supply/AC/online", 1); + bs->AcOnLine = TRUE; + if (!d) return STATUS_SUCCESS; - for (;;) + while ((de = readdir(d))) { - sprintf(path, "/sys/class/power_supply/BAT%u/status", i); - if (!get_sys_str(path, s)) break; - bs->Charging |= (strcmp(s, "Charging\n") == 0); - bs->Discharging |= (strcmp(s, "Discharging\n") == 0); - bs->BatteryPresent = TRUE; - i++; - } + if (strcmp(de->d_name, ".") == 0 || strcmp(de->d_name, "..") == 0) continue; + if (snprintf(path, sizeof(path), "/sys/class/power_supply/%s", de->d_name) < 0) continue; + if (get_sys_str(path, "scope", s) && strcmp(s, "Device\n") == 0) continue; + if (!get_sys_str(path, "type", s)) continue; - if (bs->BatteryPresent) - { - voltage = get_sys_int("/sys/class/power_supply/BAT0/voltage_now", 0); - bs->MaxCapacity = get_sys_int("/sys/class/power_supply/BAT0/charge_full", 0) * voltage / 1e9; - bs->RemainingCapacity = get_sys_int("/sys/class/power_supply/BAT0/charge_now", 0) * voltage / 1e9; - bs->Rate = -get_sys_int("/sys/class/power_supply/BAT0/current_now", 0) * voltage / 1e9; - if (!bs->Charging && (LONG)bs->Rate < 0) - bs->EstimatedTime = 3600 * bs->RemainingCapacity / -(LONG)bs->Rate; - else - bs->EstimatedTime = ~0u; + if (strcmp(s, "Mains\n") == 0) + { + if (!get_sys_str(path, "online", s)) continue; + if (found_ac) + { + FIXME("Multiple mains found, only reporting on the first\n"); + } + else + { + bs->AcOnLine = atoi(s); + found_ac = TRUE; + } + } + else if (strcmp(s, "Battery\n") == 0) + { + if (!get_sys_str(path, "status", s)) continue; + if (bs->BatteryPresent) + { + FIXME("Multiple batteries found, only reporting on the first\n"); + } + else + { + bs->Charging = (strcmp(s, "Charging\n") == 0); + bs->Discharging = (strcmp(s, "Discharging\n") == 0); + bs->BatteryPresent = TRUE; + voltage = get_sys_int(path, "voltage_now"); + bs->MaxCapacity = get_sys_int(path, "charge_full") * voltage / 1e9; + bs->RemainingCapacity = get_sys_int(path, "charge_now") * voltage / 1e9; + bs->Rate = -get_sys_int(path, "current_now") * voltage / 1e9; + if (!bs->Charging && (LONG)bs->Rate < 0) + bs->EstimatedTime = 3600 * bs->RemainingCapacity / -(LONG)bs->Rate; + else + bs->EstimatedTime = ~0u; + } + } } + closedir(d); return STATUS_SUCCESS; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/134
On Wed Jun 15 14:14:20 2022 +0000, Huw Davies wrote:
Sebastian Reichel replied: [1](https://www.winehq.org/pipermail/wine-devel/2022-June/220112.html)
/sys/class/power_supply/*/scope is set to 'Device' for all batteries, that are not powering the main system. There can still be multiple batteries; for example Thinkpads in the 2014-2018 era used to have an internal and a removable battery. For a single "X %/hours left" info the data from all 'System' level batteries must be aggregated. Benjamin Berg also mentioned UPower: [2](https://www.winehq.org/pipermail/wine-devel/2022-June/220110.html)
Wouldn't it make sense for Wine to use the UPower provided DisplayDevice that can be queried through DBus? To which Sebastian replied: UPower does the required data aggregation for the 'DisplayDevice'. I don't know enough about the Wine codebase to recommend for or against using UPower. So it looks like the patch's general idea is correct. I would suggest adding a `FIXME()` if a second system battery is encountered. Also, `bs->BatteryPresent` could be used instead of the `found_battery` variable. Thanks for the feedback. I have added FIXMEs and removed the found_battery variable as you suggested.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/134#note_2112
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/134
Alexandre Julliard (@julliard) commented about dlls/ntdll/unix/system.c:
return cmz; }
-static const char * get_sys_str(const char *path, char *s) +static const char * get_sys_str(const char *dirname, const char *basename, char *s) { - FILE *f = fopen(path, "r"); + char path[64]; + FILE *f; const char *ret = NULL;
- if (f) + if (snprintf(path, sizeof(path), "%s/%s", dirname, basename) < 0) return NULL;
snprintf doesn't return -1 on overflow (which is the only failure you care about here). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/134#note_2141
Alexandre Julliard (@julliard) commented about dlls/ntdll/unix/system.c:
- bs->AcOnLine = get_sys_int("/sys/class/power_supply/AC/online", 1); + bs->AcOnLine = TRUE; + if (!d) return STATUS_SUCCESS;
- for (;;) + while ((de = readdir(d))) { - sprintf(path, "/sys/class/power_supply/BAT%u/status", i); - if (!get_sys_str(path, s)) break; - bs->Charging |= (strcmp(s, "Charging\n") == 0); - bs->Discharging |= (strcmp(s, "Discharging\n") == 0); - bs->BatteryPresent = TRUE; - i++; - } + if (strcmp(de->d_name, ".") == 0 || strcmp(de->d_name, "..") == 0) continue; + if (snprintf(path, sizeof(path), "/sys/class/power_supply/%s", de->d_name) < 0) continue; Same here.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/134#note_2142
participants (4)
-
Alex Henrie -
Alex Henrie (＠alexhenrie)
-
Alexandre Julliard (＠julliard)
-
Huw Davies (＠huw)