-
-
Notifications
You must be signed in to change notification settings - Fork 614
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
Fix AC/battery detection logic on FreeBSD. #1972
Conversation
The logic for the detection of the battery/AC line was not completely correct for FreeBSD. Launching conky on console shows the following: Cannot read sysctl "hw.acpi.battery.time" Cannot read sysctl "hw.acpi.battery.life" Cannot read sysctl "hw.acpi.battery.state" Cannot read sysctl "hw.acpi.acline" Unknown battery state 8! In a PC, the hw.acpi.battery MIB does not exist. Also, the hw.acpi.acline is only present if supported by the hardware. In addition, some variables were used uninitialized and that causes strange behavior: in a PC it showed it worked on battery and the % of charge was an ridiculous big number. This patch addresses the issue. It fixes the problem in the PC. It has also being tested in a laptop running FreeBSD current plugin and unplugging the AC line and also snatching the battery mercilessly to see if something breaks.
✅ Deploy Preview for conkyweb canceled.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall it's good, but since you're making changes here we should replace those fprintf()
calls with NORM_ERR
instead, if you wouldn't mind.
src/freebsd.cc
Outdated
|
||
if (battery_present) { | ||
if (battime && GETSYSCTL("hw.acpi.battery.time", *battime)) { | ||
fprintf(stderr, "Cannot read sysctl \"hw.acpi.battery.time\"\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fprintf(stderr, "Cannot read sysctl \"hw.acpi.battery.time\"\n"); | |
NORM_ERR("Cannot read sysctl \"hw.acpi.battery.time\""); |
src/freebsd.cc
Outdated
fprintf(stderr, "Cannot read sysctl \"hw.acpi.battery.time\"\n"); | ||
} | ||
if (batcapacity && GETSYSCTL("hw.acpi.battery.life", *batcapacity)) { | ||
fprintf(stderr, "Cannot read sysctl \"hw.acpi.battery.life\"\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fprintf(stderr, "Cannot read sysctl \"hw.acpi.battery.life\"\n"); | |
NORM_ERR("Cannot read sysctl \"hw.acpi.battery.life\""); |
src/freebsd.cc
Outdated
fprintf(stderr, "Cannot read sysctl \"hw.acpi.battery.life\"\n"); | ||
} | ||
if (batstate && GETSYSCTL("hw.acpi.battery.state", *batstate)) { | ||
fprintf(stderr, "Cannot read sysctl \"hw.acpi.battery.state\"\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fprintf(stderr, "Cannot read sysctl \"hw.acpi.battery.state\"\n"); | |
NORM_ERR("Cannot read sysctl \"hw.acpi.battery.state\""); |
Thank you for the suggestion. I didn't know about |
The logic for the detection of the battery/AC line was not completely correct for FreeBSD. Launching conky on console shows the following:
Cannot read sysctl "hw.acpi.battery.time"
Cannot read sysctl "hw.acpi.battery.life"
Cannot read sysctl "hw.acpi.battery.state"
Cannot read sysctl "hw.acpi.acline"
Unknown battery state 8!
In a PC, the hw.acpi.battery MIB does not exist.
Also, the hw.acpi.acline is only present if supported by the hardware. In addition, some variables were used uninitialized and that causes strange behavior: in a PC it showed it worked on battery and the % of charge was an ridiculous big number.
This patch addresses the issue. It fixes the problem in the PC. It has also being tested in a laptop running FreeBSD current plugin and unplugging the AC line and also snatching the battery mercilessly to see if something breaks.
Checklist
doc/
has been updatedDescription