Add support for battery monitoring #612
Conversation
…ryStatus variables are null
|
Thanks, when possible, I'd like to avoid WMI. It's possible to request this information through Win32 API as well:: |
|
I've tried to get battery information through Win32 before, but it doesn't work. Used this code as a reference: https://docs.microsoft.com/en-us/windows/win32/power/enumerating-battery-devices. |
It should definitely be possible to use this to get the battery information. There are some (C#) projects on GitHub that use it and return the correct battery information. |
|
I already discovered the bug in my code that was preventing me from getting battery info through Win32, it was hard to find. In a moment I will send the commit that replaces WMI with Win32. |
…FORMATION" must be 4, not 3.
|
Hi. Is there something I should fix before merging? |
LibreHardwareMonitor/UI/TypeNode.cs
Outdated
| @@ -31,6 +31,10 @@ public TypeNode(SensorType sensorType, Identifier parentId, PersistentSettings s | |||
| Image = Utilities.EmbeddedResources.GetImage("voltage.png"); | |||
| Text = "Currents"; | |||
| break; | |||
| case SensorType.Energy: | |||
| Image = Utilities.EmbeddedResources.GetImage("voltage.png"); | |||
There was a problem hiding this comment.
You're right. Changed
| private readonly Sensor _remainingCapacity; | ||
| private readonly Sensor _chargeDischargeRate; | ||
| private readonly Sensor _degradationPercentage; | ||
|
|
| using System.IO; | ||
| namespace LibreHardwareMonitor.Hardware.Battery | ||
| { | ||
| internal class BatteryGroup : IGroup |
There was a problem hiding this comment.
Format document / sort fields.
| battery.Close(); | ||
| } | ||
| } | ||
| public string GetReport() => null; |
…Charge/Discharge Current" when necessary. Because that value depends of ch./disch. rate sensor.
| internal struct BatteryReportInfo | ||
| { | ||
| public string Name { get; set; } | ||
| public uint Tag { get; set; } | ||
| public SafeFileHandle BatteryHandle { get; set; } | ||
| public Kernel32.BATTERY_INFORMATION MoreInformation { get; set; } | ||
| } |
There was a problem hiding this comment.
Thanks, instead of this, can we perhaps abstract some of the stuff in ctor and call that also in GetReport?
There was a problem hiding this comment.
Yes. We could also move those properties into the Battery class and call them from GetReport(). It will be necessary to add more battery properties. Properties would be updated in the Battery constructor and in the Update() method.
| // Limit battery search to 100 batteries | ||
| for (uint idev = 0; idev < 100; idev++) |
There was a problem hiding this comment.
While I seriously doubt there would be more than 100 batteries, this should just loop until ERROR_NO_MORE_ITEMS is the last error.
There was a problem hiding this comment.
Yes, you're right. That's from the C++ code I've used as a reference. It's fixed now.
| IntPtr.Zero); | ||
| if (Marshal.GetLastWin32Error() == SetupApi.ERROR_INSUFFICIENT_BUFFER) | ||
| { | ||
| IntPtr pdidd = Kernel32.LocalAlloc(Kernel32.LPTR, cbRequired); |
There was a problem hiding this comment.
Marshal.AllocHGlobal and below Marshal.FreeHGlobal?
There was a problem hiding this comment.
No. Because it IS necessary (in this case) to pass the LPTR (0x0000 | 0x0040) flag in LocalAlloc(). Marshal.AllocHGlobal() calls LocalAlloc(), but it passes a LMEM_FIXED (0x0000) flag. If I use the Marshal.AllocHGlobal() method, LHM crashes inmediately.
| if (Marshal.GetLastWin32Error() == SetupApi.ERROR_INSUFFICIENT_BUFFER) | ||
| { | ||
| IntPtr pdidd = Kernel32.LocalAlloc(Kernel32.LPTR, cbRequired); | ||
| SetupApi.SP_DEVICE_INTERFACE_DETAIL_DATA didd = SetupApi.SP_DEVICE_INTERFACE_DETAIL_DATA.Default; |
There was a problem hiding this comment.
You don't seem to be doing anything with SP_DEVICE_INTERFACE_DETAIL_DATA, might as well remove this.
There was a problem hiding this comment.
Also force SetupDiGetDeviceInterfaceDetail to unicode variant so you can remove CharSet.Auto stuff.
There was a problem hiding this comment.
You don't seem to be doing anything with SP_DEVICE_INTERFACE_DETAIL_DATA, might as well remove this.
I've removed the "didd" variable. SP_DEVICE_INTERFACE_DETAIL_DATA is necessary to do the conversion to a pointer.
Also force SetupDiGetDeviceInterfaceDetail to unicode variant so you can remove CharSet.Auto stuff.
Done
There was a problem hiding this comment.
Thanks @EMN-CSharp, I have reformatted it a bit, can you check if everything is still working? Worked OK here but one more is always good, then I'll merge.
There was a problem hiding this comment.
Everything worked OK here:
…viceInterfaceDetail
|
I'd suggest to merge the "Power supplies" and "Batteries" hardware categories. "Power sources", maybe? |
Wouldn't be as clear maybe when you have both and just see a type number. Feel free to open an issue for it and discuss this 😁 |
|
Thank you @PhyxionNL !!! |
Thank you for your contribution, it's appreciated 👍 |
This PR adds support for monitoring values like voltage, current, charge level, charge/discharge rate, degradation level and capacities. Resolves #599
Before:
After:
