diff --git a/agent/battery/battery.go b/agent/battery/battery.go index a87b86bb..af35e002 100644 --- a/agent/battery/battery.go +++ b/agent/battery/battery.go @@ -1,6 +1,13 @@ -// Package battery provides functions to check if the system has a battery and return the charge state and percentage. +// Package battery provides battery information for the host and connected devices. package battery +import ( + "errors" + "sort" + "strconv" + "strings" +) + const ( stateUnknown uint8 = iota stateEmpty @@ -9,3 +16,55 @@ const ( stateDischarging stateIdle ) + +// Battery is a readable battery reported by the operating system. +type Battery struct { + Name string + Percent uint8 + State uint8 + FullChargeCapacity uint64 + HasFullChargeCapacity bool + System bool +} + +var errNoBatteries = errors.New("no readable batteries") + +// normalizeBatteries supplies stable fallback names and disambiguates duplicates. +func normalizeBatteries(batteries []Battery) []Battery { + nameCounts := make(map[string]int, len(batteries)) + for i := range batteries { + name := strings.TrimSpace(batteries[i].Name) + if name == "" { + name = "Battery " + strconv.Itoa(i+1) + } + nameCounts[name]++ + if nameCounts[name] > 1 { + name += " (" + strconv.Itoa(nameCounts[name]) + ")" + } + batteries[i].Name = name + } + return batteries +} + +// Primary returns the representative battery. Reported full-charge capacity wins, +// then system-scoped devices, then name for deterministic ties. +func Primary(batteries []Battery) (Battery, bool) { + if len(batteries) == 0 { + return Battery{}, false + } + ordered := append([]Battery(nil), batteries...) + sort.SliceStable(ordered, func(i, j int) bool { + a, b := ordered[i], ordered[j] + if a.HasFullChargeCapacity != b.HasFullChargeCapacity { + return a.HasFullChargeCapacity + } + if a.HasFullChargeCapacity && a.FullChargeCapacity != b.FullChargeCapacity { + return a.FullChargeCapacity > b.FullChargeCapacity + } + if a.System != b.System { + return a.System + } + return a.Name < b.Name + }) + return ordered[0], true +} diff --git a/agent/battery/battery_darwin.go b/agent/battery/battery_darwin.go index e32e535d..f562c573 100644 --- a/agent/battery/battery_darwin.go +++ b/agent/battery/battery_darwin.go @@ -3,11 +3,7 @@ package battery import ( - "errors" - "log/slog" - "math" "os/exec" - "sync" "howett.net/plist" ) @@ -35,62 +31,46 @@ func readMacBatteries() ([]macBattery, error) { return batteries, nil } -// HasReadableBattery checks if the system has a battery and returns true if it does. -var HasReadableBattery = sync.OnceValue(func() bool { - systemHasBattery := false - batteries, err := readMacBatteries() - slog.Debug("Batteries", "batteries", batteries, "err", err) - for _, bat := range batteries { - if bat.MaxCapacity > 0 { - systemHasBattery = true - break - } - } - return systemHasBattery -}) +func HasReadableBattery() bool { + batteries, _ := GetBatteryStats() + return len(batteries) > 0 +} -// GetBatteryStats returns the current battery percent and charge state. -// Uses CurrentCapacity/MaxCapacity to match the value macOS displays. -func GetBatteryStats() (batteryPercent uint8, batteryState uint8, err error) { - if !HasReadableBattery() { - return batteryPercent, batteryState, errors.ErrUnsupported - } +// GetBatteryStats returns every readable battery reported by macOS. +func GetBatteryStats() ([]Battery, error) { batteries, err := readMacBatteries() + if err != nil { + return nil, err + } if len(batteries) == 0 { - return batteryPercent, batteryState, errors.New("no batteries") + return nil, errNoBatteries } - - totalCapacity := 0 - totalCharge := 0 - batteryState = math.MaxUint8 - + result := make([]Battery, 0, len(batteries)) for _, bat := range batteries { - if bat.MaxCapacity == 0 { + if bat.MaxCapacity <= 0 { // skip ghost batteries with 0 capacity // https://github.com/distatus/battery/issues/34 continue } - totalCapacity += bat.MaxCapacity - totalCharge += min(bat.CurrentCapacity, bat.MaxCapacity) - + percent := min(max(float64(bat.CurrentCapacity)/float64(bat.MaxCapacity)*100, 0), 100) + state := stateUnknown switch { case !bat.ExternalConnected: - batteryState = stateDischarging + state = stateDischarging case bat.IsCharging: - batteryState = stateCharging + state = stateCharging case bat.CurrentCapacity == 0: - batteryState = stateEmpty + state = stateEmpty case !bat.FullyCharged: - batteryState = stateIdle + state = stateIdle default: - batteryState = stateFull + state = stateFull } + result = append(result, Battery{Name: "Primary", Percent: uint8(percent), State: state, + FullChargeCapacity: uint64(bat.MaxCapacity), HasFullChargeCapacity: true, System: true}) } - - if totalCapacity == 0 || batteryState == math.MaxUint8 { - return batteryPercent, batteryState, errors.New("no battery capacity") + if len(result) == 0 { + return nil, errNoBatteries } - - batteryPercent = uint8(float64(totalCharge) / float64(totalCapacity) * 100) - return batteryPercent, batteryState, nil + return normalizeBatteries(result), nil } diff --git a/agent/battery/battery_linux.go b/agent/battery/battery_linux.go index 84414a19..2935d5aa 100644 --- a/agent/battery/battery_linux.go +++ b/agent/battery/battery_linux.go @@ -3,58 +3,19 @@ package battery import ( - "errors" - "log/slog" - "math" "os" "path/filepath" "strconv" - "sync" "github.com/henrygd/beszel/agent/utils" ) -// getBatteryPaths returns the paths of all batteries in /sys/class/power_supply -var getBatteryPaths func() ([]string, error) +var batteryRoot = "/sys/class/power_supply" -// HasReadableBattery checks if the system has a battery and returns true if it does. -var HasReadableBattery func() bool - -func init() { - resetBatteryState("/sys/class/power_supply") -} - -// resetBatteryState resets the sync.Once functions to a fresh state. -// Tests call this after swapping sysfsPowerSupply so the new path is picked up. -func resetBatteryState(sysfsPowerSupplyPath string) { - getBatteryPaths = sync.OnceValues(func() ([]string, error) { - entries, err := os.ReadDir(sysfsPowerSupplyPath) - if err != nil { - return nil, err - } - var paths []string - for _, e := range entries { - path := filepath.Join(sysfsPowerSupplyPath, e.Name()) - if utils.ReadStringFile(filepath.Join(path, "type")) == "Battery" { - paths = append(paths, path) - } - } - return paths, nil - }) - HasReadableBattery = sync.OnceValue(func() bool { - systemHasBattery := false - paths, err := getBatteryPaths() - for _, path := range paths { - if _, ok := utils.ReadStringFileOK(filepath.Join(path, "capacity")); ok { - systemHasBattery = true - break - } - } - if !systemHasBattery { - slog.Debug("No battery found", "err", err) - } - return systemHasBattery - }) +// HasReadableBattery reports whether collection currently finds a readable battery. +func HasReadableBattery() bool { + batteries, _ := GetBatteryStats() + return len(batteries) > 0 } func parseSysfsState(status string) uint8 { @@ -74,26 +35,18 @@ func parseSysfsState(status string) uint8 { } } -// GetBatteryStats returns the current battery percent and charge state. -// Reads /sys/class/power_supply/*/capacity directly so the kernel-reported -// value is used, which is always 0-100 and matches what the OS displays. -func GetBatteryStats() (batteryPercent uint8, batteryState uint8, err error) { - if !HasReadableBattery() { - return batteryPercent, batteryState, errors.ErrUnsupported - } - paths, err := getBatteryPaths() +// GetBatteryStats re-enumerates power supplies and returns every readable battery. +func GetBatteryStats() ([]Battery, error) { + entries, err := os.ReadDir(batteryRoot) if err != nil { - return batteryPercent, batteryState, err + return nil, err } - if len(paths) == 0 { - return batteryPercent, batteryState, errors.New("no batteries") - } - - batteryState = math.MaxUint8 - totalPercent := 0 - count := 0 - - for _, path := range paths { + batteries := make([]Battery, 0, len(entries)) + for _, entry := range entries { + path := filepath.Join(batteryRoot, entry.Name()) + if utils.ReadStringFile(filepath.Join(path, "type")) != "Battery" { + continue + } capStr, ok := utils.ReadStringFileOK(filepath.Join(path, "capacity")) if !ok { continue @@ -103,19 +56,30 @@ func GetBatteryStats() (batteryPercent uint8, batteryState uint8, err error) { continue } cap = min(max(cap, 0), 100) - totalPercent += cap - count++ - - state := parseSysfsState(utils.ReadStringFile(filepath.Join(path, "status"))) - if state != stateUnknown { - batteryState = state + name := utils.ReadStringFile(filepath.Join(path, "model_name")) + if name == "" { + name = utils.ReadStringFile(filepath.Join(path, "model")) } + if name == "" { + name = entry.Name() + } + battery := Battery{ + Name: name, + Percent: uint8(cap), + State: parseSysfsState(utils.ReadStringFile(filepath.Join(path, "status"))), + System: utils.ReadStringFile(filepath.Join(path, "scope")) != "Device", + } + for _, fullName := range []string{"charge_full", "energy_full"} { + if parsed, ok := utils.ReadUintFile(filepath.Join(path, fullName)); ok && parsed > 0 { + battery.FullChargeCapacity = parsed + battery.HasFullChargeCapacity = true + break + } + } + batteries = append(batteries, battery) } - - if count == 0 || batteryState == math.MaxUint8 { - return batteryPercent, batteryState, errors.New("no battery capacity") + if len(batteries) == 0 { + return nil, errNoBatteries } - - batteryPercent = uint8(totalPercent / count) - return batteryPercent, batteryState, nil + return normalizeBatteries(batteries), nil } diff --git a/agent/battery/battery_linux_test.go b/agent/battery/battery_linux_test.go index 15fed19c..01fb2a79 100644 --- a/agent/battery/battery_linux_test.go +++ b/agent/battery/battery_linux_test.go @@ -8,204 +8,102 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -// setupFakeSysfs creates a temporary sysfs-like tree under t.TempDir(), -// swaps sysfsPowerSupply, resets the sync.Once caches, and restores -// everything on cleanup. Returns a helper to create battery directories. -func setupFakeSysfs(t *testing.T) (tmpDir string, addBattery func(name, capacity, status string)) { +type fakeBattery struct{ id, name, capacity, status, full, scope string } + +func setupFakeSysfs(t *testing.T) (string, func(fakeBattery)) { t.Helper() - - tmp := t.TempDir() - resetBatteryState(tmp) - - write := func(path, content string) { + root := t.TempDir() + previousRoot := batteryRoot + batteryRoot = root + t.Cleanup(func() { batteryRoot = previousRoot }) + write := func(path, value string) { t.Helper() - dir := filepath.Dir(path) - if err := os.MkdirAll(dir, 0o755); err != nil { - t.Fatal(err) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, []byte(value), 0o644)) + } + add := func(b fakeBattery) { + t.Helper() + dir := filepath.Join(root, b.id) + write(filepath.Join(dir, "type"), "Battery") + if b.capacity != "" { + write(filepath.Join(dir, "capacity"), b.capacity) } - if err := os.WriteFile(path, []byte(content), 0o644); err != nil { - t.Fatal(err) + write(filepath.Join(dir, "status"), b.status) + if b.name != "" { + write(filepath.Join(dir, "model_name"), b.name) + } + if b.full != "" { + write(filepath.Join(dir, "energy_full"), b.full) + } + if b.scope != "" { + write(filepath.Join(dir, "scope"), b.scope) } } - - addBattery = func(name, capacity, status string) { - t.Helper() - batDir := filepath.Join(tmp, name) - write(filepath.Join(batDir, "type"), "Battery") - write(filepath.Join(batDir, "capacity"), capacity) - write(filepath.Join(batDir, "status"), status) - } - - return tmp, addBattery + return root, add } func TestParseSysfsState(t *testing.T) { - tests := []struct { - input string - want uint8 - }{ - {"Empty", stateEmpty}, - {"Full", stateFull}, - {"Charging", stateCharging}, - {"Discharging", stateDischarging}, - {"Not charging", stateIdle}, - {"", stateUnknown}, - {"SomethingElse", stateUnknown}, - } - for _, tt := range tests { - assert.Equal(t, tt.want, parseSysfsState(tt.input), "parseSysfsState(%q)", tt.input) - } + assert.Equal(t, stateEmpty, parseSysfsState("Empty")) + assert.Equal(t, stateFull, parseSysfsState("Full")) + assert.Equal(t, stateCharging, parseSysfsState("Charging")) + assert.Equal(t, stateDischarging, parseSysfsState("Discharging")) + assert.Equal(t, stateIdle, parseSysfsState("Not charging")) + assert.Equal(t, stateUnknown, parseSysfsState("SomethingElse")) } -func TestGetBatteryStats_SingleBattery(t *testing.T) { - _, addBattery := setupFakeSysfs(t) - addBattery("BAT0", "72", "Discharging") - - pct, state, err := GetBatteryStats() - assert.NoError(t, err) - assert.Equal(t, uint8(72), pct) - assert.Equal(t, stateDischarging, state) +func TestGetBatteryStatsMultipleNamedAndPrimary(t *testing.T) { + _, add := setupFakeSysfs(t) + add(fakeBattery{id: "BAT0", name: "Primary", capacity: "105", status: "Charging", full: "5000", scope: "System"}) + add(fakeBattery{id: "hidpp_battery_0", name: "MX Keys S", capacity: "55", status: "Unknown", full: "900", scope: "Device"}) + batteries, err := GetBatteryStats() + require.NoError(t, err) + require.Len(t, batteries, 2) + assert.Equal(t, "Primary", batteries[0].Name) + assert.Equal(t, uint8(100), batteries[0].Percent) + assert.Equal(t, stateUnknown, batteries[1].State) + primary, ok := Primary(batteries) + require.True(t, ok) + assert.Equal(t, "Primary", primary.Name) } -func TestGetBatteryStats_MultipleBatteries(t *testing.T) { - _, addBattery := setupFakeSysfs(t) - addBattery("BAT0", "80", "Charging") - addBattery("BAT1", "40", "Charging") - - pct, state, err := GetBatteryStats() - assert.NoError(t, err) - // average of 80 and 40 = 60 - assert.EqualValues(t, 60, pct) - assert.Equal(t, stateCharging, state) +func TestGetBatteryStatsFallbackDuplicatesAndUnreadable(t *testing.T) { + root, add := setupFakeSysfs(t) + add(fakeBattery{id: "BAT0", name: "Keyboard", capacity: "80", status: "Discharging"}) + add(fakeBattery{id: "BAT1", name: "Keyboard", capacity: "-4", status: "SomethingWeird"}) + add(fakeBattery{id: "BAT2", capacity: "not-a-number", status: "Charging"}) + add(fakeBattery{id: "BAT3", capacity: "42", status: "Full"}) + ac := filepath.Join(root, "AC0") + require.NoError(t, os.MkdirAll(ac, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(ac, "type"), []byte("Mains"), 0o644)) + batteries, err := GetBatteryStats() + require.NoError(t, err) + require.Len(t, batteries, 3) + assert.Equal(t, "Keyboard", batteries[0].Name) + assert.Equal(t, "Keyboard (2)", batteries[1].Name) + assert.Equal(t, uint8(0), batteries[1].Percent) + assert.Equal(t, "BAT3", batteries[2].Name) } -func TestGetBatteryStats_FullBattery(t *testing.T) { - _, addBattery := setupFakeSysfs(t) - addBattery("BAT0", "100", "Full") - - pct, state, err := GetBatteryStats() - assert.NoError(t, err) - assert.Equal(t, uint8(100), pct) - assert.Equal(t, stateFull, state) -} - -func TestGetBatteryStats_CapacityClamped(t *testing.T) { - _, addBattery := setupFakeSysfs(t) - addBattery("BAT0", "105", "Charging") - - pct, state, err := GetBatteryStats() - assert.NoError(t, err) - assert.Equal(t, uint8(100), pct) - assert.Equal(t, stateCharging, state) -} - -func TestGetBatteryStats_EmptyBattery(t *testing.T) { - _, addBattery := setupFakeSysfs(t) - addBattery("BAT0", "0", "Empty") - - pct, state, err := GetBatteryStats() - assert.NoError(t, err) - assert.Equal(t, uint8(0), pct) - assert.Equal(t, stateEmpty, state) -} - -func TestGetBatteryStats_NotCharging(t *testing.T) { - _, addBattery := setupFakeSysfs(t) - addBattery("BAT0", "80", "Not charging") - - pct, state, err := GetBatteryStats() - assert.NoError(t, err) - assert.Equal(t, uint8(80), pct) - assert.Equal(t, stateIdle, state) -} - -func TestGetBatteryStats_NoBatteries(t *testing.T) { - setupFakeSysfs(t) // empty directory, no batteries - - _, _, err := GetBatteryStats() +func TestGetBatteryStatsHotPlugReenumerates(t *testing.T) { + _, add := setupFakeSysfs(t) + _, err := GetBatteryStats() assert.Error(t, err) -} - -func TestGetBatteryStats_NonBatterySupplyIgnored(t *testing.T) { - tmp, addBattery := setupFakeSysfs(t) - - // Add a real battery - addBattery("BAT0", "55", "Charging") - - // Add an AC adapter (type != Battery) - should be ignored - acDir := filepath.Join(tmp, "AC0") - if err := os.MkdirAll(acDir, 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(acDir, "type"), []byte("Mains"), 0o644); err != nil { - t.Fatal(err) - } - - pct, state, err := GetBatteryStats() - assert.NoError(t, err) - assert.Equal(t, uint8(55), pct) - assert.Equal(t, stateCharging, state) -} - -func TestGetBatteryStats_InvalidCapacitySkipped(t *testing.T) { - tmp, addBattery := setupFakeSysfs(t) - - // One battery with valid capacity - addBattery("BAT0", "90", "Discharging") - - // Another with invalid capacity text - badDir := filepath.Join(tmp, "BAT1") - if err := os.MkdirAll(badDir, 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(badDir, "type"), []byte("Battery"), 0o644); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(badDir, "capacity"), []byte("not-a-number"), 0o644); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(badDir, "status"), []byte("Discharging"), 0o644); err != nil { - t.Fatal(err) - } - - pct, _, err := GetBatteryStats() - assert.NoError(t, err) - // Only BAT0 counted - assert.Equal(t, uint8(90), pct) -} - -func TestGetBatteryStats_UnknownStatusOnly(t *testing.T) { - _, addBattery := setupFakeSysfs(t) - addBattery("BAT0", "50", "SomethingWeird") - - _, _, err := GetBatteryStats() - assert.Error(t, err) -} - -func TestHasReadableBattery_True(t *testing.T) { - _, addBattery := setupFakeSysfs(t) - addBattery("BAT0", "50", "Charging") - + assert.False(t, HasReadableBattery()) + add(fakeBattery{id: "BAT0", capacity: "64", status: "Discharging"}) + batteries, err := GetBatteryStats() + require.NoError(t, err) assert.True(t, HasReadableBattery()) + require.Len(t, batteries, 1) + assert.Equal(t, uint8(64), batteries[0].Percent) } -func TestHasReadableBattery_False(t *testing.T) { - setupFakeSysfs(t) // no batteries - - assert.False(t, HasReadableBattery()) -} - -func TestHasReadableBattery_NoCapacityFile(t *testing.T) { - tmp, _ := setupFakeSysfs(t) - - // Battery dir with type file but no capacity file - batDir := filepath.Join(tmp, "BAT0") - err := os.MkdirAll(batDir, 0o755) - assert.NoError(t, err) - err = os.WriteFile(filepath.Join(batDir, "type"), []byte("Battery"), 0o644) - assert.NoError(t, err) - +func TestGetBatteryStatsNoReadableCapacity(t *testing.T) { + _, add := setupFakeSysfs(t) + add(fakeBattery{id: "BAT0", status: "Charging"}) + _, err := GetBatteryStats() + assert.Error(t, err) assert.False(t, HasReadableBattery()) } diff --git a/agent/battery/battery_stub.go b/agent/battery/battery_stub.go index 08cee52d..b7302ab2 100644 --- a/agent/battery/battery_stub.go +++ b/agent/battery/battery_stub.go @@ -8,6 +8,6 @@ func HasReadableBattery() bool { return false } -func GetBatteryStats() (uint8, uint8, error) { - return 0, 0, errors.ErrUnsupported +func GetBatteryStats() ([]Battery, error) { + return nil, errors.ErrUnsupported } diff --git a/agent/battery/battery_test.go b/agent/battery/battery_test.go new file mode 100644 index 00000000..9280eae3 --- /dev/null +++ b/agent/battery/battery_test.go @@ -0,0 +1,35 @@ +package battery + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPrimarySelection(t *testing.T) { + tests := []struct { + name string + bats []Battery + want string + }{ + {"largest reported capacity", []Battery{{Name: "Small", FullChargeCapacity: 20, HasFullChargeCapacity: true, System: true}, {Name: "Large", FullChargeCapacity: 80, HasFullChargeCapacity: true}}, "Large"}, + {"reported ranks over missing", []Battery{{Name: "Unknown", System: true}, {Name: "Known", FullChargeCapacity: 1, HasFullChargeCapacity: true}}, "Known"}, + {"system wins capacity tie", []Battery{{Name: "Peripheral", FullChargeCapacity: 50, HasFullChargeCapacity: true}, {Name: "System", FullChargeCapacity: 50, HasFullChargeCapacity: true, System: true}}, "System"}, + {"name resolves final tie", []Battery{{Name: "Zed"}, {Name: "Alpha"}}, "Alpha"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, ok := Primary(tt.bats) + require.True(t, ok) + assert.Equal(t, tt.want, got.Name) + }) + } + _, ok := Primary(nil) + assert.False(t, ok) +} + +func TestNormalizeBatteriesFallbackNames(t *testing.T) { + bats := normalizeBatteries([]Battery{{}, {}, {Name: "Mouse"}, {Name: "Mouse"}}) + assert.Equal(t, []string{"Battery 1", "Battery 2", "Mouse", "Mouse (2)"}, []string{bats[0].Name, bats[1].Name, bats[2].Name, bats[3].Name}) +} diff --git a/agent/battery/battery_windows.go b/agent/battery/battery_windows.go index fd29b1cd..4bd3b1fe 100644 --- a/agent/battery/battery_windows.go +++ b/agent/battery/battery_windows.go @@ -7,9 +7,6 @@ package battery import ( "errors" - "log/slog" - "math" - "sync" "syscall" "unsafe" @@ -79,7 +76,7 @@ var ( setupDiDestroyDeviceInfoList = setupapi.NewProc("SetupDiDestroyDeviceInfoList") ) -// winBatteryGet reads one battery by index. Returns (fullCapacity, currentCapacity, state, error). +// winBatteryGet reads one battery by index. // Returns error == errNotFound when there are no more batteries. var errNotFound = errors.New("no more batteries") @@ -122,7 +119,7 @@ func readWinBatteryState(powerState uint32) uint8 { } } -func winBatteryGet(idx int) (full, current uint32, state uint8, err error) { +func winBatteryGet(idx int) (Battery, error) { hdev, err := setupDiSetup( setupDiGetClassDevsW, 4, @@ -132,7 +129,7 @@ func winBatteryGet(idx int) (full, current uint32, state uint8, err error) { 0, 0, ) if err != nil { - return 0, 0, stateUnknown, err + return Battery{}, err } defer syscall.SyscallN(setupDiDestroyDeviceInfoList.Addr(), hdev) @@ -148,10 +145,10 @@ func winBatteryGet(idx int) (full, current uint32, state uint8, err error) { 0, ) if errno == 259 { // ERROR_NO_MORE_ITEMS - return 0, 0, stateUnknown, errNotFound + return Battery{}, errNotFound } if errno != 0 { - return 0, 0, stateUnknown, errno + return Battery{}, errno } var cbRequired uint32 @@ -165,7 +162,7 @@ func winBatteryGet(idx int) (full, current uint32, state uint8, err error) { 0, ) if errno != 0 && errno != 122 { // ERROR_INSUFFICIENT_BUFFER - return 0, 0, stateUnknown, errno + return Battery{}, errno } didd := make([]uint16, cbRequired/2) cbSize := (*uint32)(unsafe.Pointer(&didd[0])) @@ -185,7 +182,7 @@ func winBatteryGet(idx int) (full, current uint32, state uint8, err error) { 0, ) if errno != 0 { - return 0, 0, stateUnknown, errno + return Battery{}, errno } devicePath := &didd[2:][0] @@ -199,7 +196,7 @@ func winBatteryGet(idx int) (full, current uint32, state uint8, err error) { 0, ) if err != nil { - return 0, 0, stateUnknown, err + return Battery{}, err } defer windows.CloseHandle(handle) @@ -216,7 +213,7 @@ func winBatteryGet(idx int) (full, current uint32, state uint8, err error) { &dwOut, nil, ) if err != nil || bqi.BatteryTag == 0 { - return 0, 0, stateUnknown, errors.New("battery tag not returned") + return Battery{}, errors.New("battery tag not returned") } var bi batteryInformation @@ -229,7 +226,21 @@ func winBatteryGet(idx int) (full, current uint32, state uint8, err error) { uint32(unsafe.Sizeof(bi)), &dwOut, nil, ); err != nil { - return 0, 0, stateUnknown, err + return Battery{}, err + } + + // BatteryDeviceName is optional, so retain the deterministic fallback on error. + name := "" + nameQuery := bqi + nameQuery.InformationLevel = 4 // BatteryDeviceName + nameBuffer := make([]uint16, 128) + if err := windows.DeviceIoControl( + handle, 2703428, + (*byte)(unsafe.Pointer(&nameQuery)), uint32(unsafe.Sizeof(nameQuery)), + (*byte)(unsafe.Pointer(&nameBuffer[0])), uint32(len(nameBuffer)*2), + &dwOut, nil, + ); err == nil { + name = windows.UTF16ToString(nameBuffer) } bws := batteryWaitStatus{BatteryTag: bqi.BatteryTag} @@ -243,56 +254,38 @@ func winBatteryGet(idx int) (full, current uint32, state uint8, err error) { uint32(unsafe.Sizeof(bs)), &dwOut, nil, ); err != nil { - return 0, 0, stateUnknown, err + return Battery{}, err } - if bs.Capacity == 0xffffffff { // BATTERY_UNKNOWN_CAPACITY - return 0, 0, stateUnknown, errors.New("battery capacity unknown") + if bs.Capacity == 0xffffffff || bi.FullChargedCapacity == 0 || bi.FullChargedCapacity == 0xffffffff { + return Battery{}, errors.New("battery capacity unknown") } - - return bi.FullChargedCapacity, bs.Capacity, readWinBatteryState(bs.PowerState), nil + percent := min(float64(bs.Capacity)/float64(bi.FullChargedCapacity)*100, 100) + return Battery{Name: name, Percent: uint8(percent), State: readWinBatteryState(bs.PowerState), + FullChargeCapacity: uint64(bi.FullChargedCapacity), HasFullChargeCapacity: true, System: true}, nil } // HasReadableBattery checks if the system has a battery and returns true if it does. -var HasReadableBattery = sync.OnceValue(func() bool { - systemHasBattery := false - full, _, _, err := winBatteryGet(0) - if err == nil && full > 0 { - systemHasBattery = true - } - if !systemHasBattery { - slog.Debug("No battery found", "err", err) - } - return systemHasBattery -}) - -// GetBatteryStats returns the current battery percent and charge state. -func GetBatteryStats() (batteryPercent uint8, batteryState uint8, err error) { - if !HasReadableBattery() { - return batteryPercent, batteryState, errors.ErrUnsupported - } - - totalFull := uint32(0) - totalCurrent := uint32(0) - batteryState = math.MaxUint8 +func HasReadableBattery() bool { + batteries, _ := GetBatteryStats() + return len(batteries) > 0 +} +// GetBatteryStats returns every readable battery reported by Windows. +func GetBatteryStats() ([]Battery, error) { + batteries := make([]Battery, 0, 2) for i := 0; ; i++ { - full, current, state, bErr := winBatteryGet(i) + battery, bErr := winBatteryGet(i) if errors.Is(bErr, errNotFound) { break } - if bErr != nil || full == 0 { + if bErr != nil { continue } - totalFull += full - totalCurrent += min(current, full) - batteryState = state + batteries = append(batteries, battery) } - - if totalFull == 0 || batteryState == math.MaxUint8 { - return batteryPercent, batteryState, errors.New("no battery capacity") + if len(batteries) == 0 { + return nil, errNoBatteries } - - batteryPercent = uint8(float64(totalCurrent) / float64(totalFull) * 100) - return batteryPercent, batteryState, nil + return normalizeBatteries(batteries), nil } diff --git a/agent/system.go b/agent/system.go index da348b46..d19ffd14 100644 --- a/agent/system.go +++ b/agent/system.go @@ -132,9 +132,14 @@ func (a *Agent) getSystemStats(cacheTimeMs uint16) system.Stats { var systemStats system.Stats // battery - if batteryPercent, batteryState, err := battery.GetBatteryStats(); err == nil { - systemStats.Battery[0] = batteryPercent - systemStats.Battery[1] = batteryState + if batteries, err := battery.GetBatteryStats(); err == nil { + systemStats.Batteries = make(map[string]uint8, len(batteries)) + for _, device := range batteries { + systemStats.Batteries[device.Name] = device.Percent + } + if primary, ok := battery.Primary(batteries); ok { + systemStats.Battery = [2]uint8{primary.Percent, primary.State} + } } // cpu metrics diff --git a/internal/alerts/alerts.go b/internal/alerts/alerts.go index 5918cfd9..826f4d68 100644 --- a/internal/alerts/alerts.go +++ b/internal/alerts/alerts.go @@ -55,6 +55,7 @@ type SystemAlertStats struct { Temperatures map[string]float32 `json:"t"` LoadAvg [3]float64 `json:"la"` Battery [2]uint8 `json:"bat"` + Batteries map[string]uint8 `json:"bats"` ExtraFs map[string]SystemAlertFsStats `json:"efs"` } diff --git a/internal/alerts/alerts_system.go b/internal/alerts/alerts_system.go index 47753134..04001d98 100644 --- a/internal/alerts/alerts_system.go +++ b/internal/alerts/alerts_system.go @@ -63,7 +63,7 @@ func (am *AlertManager) HandleSystemAlerts(systemRecord *core.Record, data *syst case "GPU": val = data.Info.GpuPct case "Battery": - if data.Stats.Battery[0] == 0 { + if !hasRepresentativeBattery(data.Stats.Battery, data.Stats.Batteries) { continue } val = float64(data.Stats.Battery[0]) @@ -167,6 +167,7 @@ func (am *AlertManager) HandleSystemAlerts(systemRecord *core.Record, data *syst stat := systemStats[i] // subtract 10 seconds to give a small time buffer systemStatsCreation := stat.Created.Time().Add(-time.Second * 10) + stats = SystemAlertStats{} if err := json.Unmarshal(stat.Stats, &stats); err != nil { return err } @@ -235,6 +236,9 @@ func (am *AlertManager) HandleSystemAlerts(systemRecord *core.Record, data *syst } alert.val += maxUsage case "Battery": + if !hasRepresentativeBattery(stats.Battery, stats.Batteries) { + continue + } alert.val += float64(stats.Battery[0]) default: continue @@ -297,6 +301,10 @@ func (am *AlertManager) HandleSystemAlerts(systemRecord *core.Record, data *syst return nil } +func hasRepresentativeBattery(legacy [2]uint8, batteries map[string]uint8) bool { + return legacy != [2]uint8{} || len(batteries) > 0 +} + func (am *AlertManager) sendSystemAlert(alert SystemAlertData) { // log.Printf("Sending alert %s: val %f | count %d | threshold %f\n", alert.name, alert.val, alert.count, alert.threshold) systemName := alert.systemRecord.GetString("name") diff --git a/internal/alerts/alerts_system_test.go b/internal/alerts/alerts_system_test.go index 8c168066..dfe0e460 100644 --- a/internal/alerts/alerts_system_test.go +++ b/internal/alerts/alerts_system_test.go @@ -199,7 +199,7 @@ func TestSystemAlertsOneMin(t *testing.T) { testOneMinuteSystemAlert(t, "LoadAvg1", 4, setLoadAvgAlertValue, [3]float64{4.1, 0, 0}, [3]float64{3.9, 0, 0}) testOneMinuteSystemAlert(t, "LoadAvg5", 4, setLoadAvgAlertValue, [3]float64{0, 4.1, 0}, [3]float64{0, 3.9, 0}) testOneMinuteSystemAlert(t, "LoadAvg15", 4, setLoadAvgAlertValue, [3]float64{0, 0, 4.1}, [3]float64{0, 0, 3.9}) - testOneMinuteSystemAlert(t, "Battery", 20, setBatteryAlertValue, [2]uint8{19, 0}, [2]uint8{21, 0}) + testOneMinuteSystemAlert(t, "Battery", 20, setBatteryAlertValue, [2]uint8{0, 1}, [2]uint8{21, 0}) } func TestSystemAlertsTwoMin(t *testing.T) { diff --git a/internal/entities/system/system.go b/internal/entities/system/system.go index b8e6cfe8..996c66df 100644 --- a/internal/entities/system/system.go +++ b/internal/entities/system/system.go @@ -34,6 +34,7 @@ type Stats struct { MaxNetworkRecv float64 `json:"nrm,omitempty" cbor:"-"` Temperatures map[string]float64 `json:"t,omitempty" cbor:"20,keyasint,omitempty"` Fans map[string]uint16 `json:"f,omitempty" cbor:"36,keyasint,omitempty"` + Batteries map[string]uint8 `json:"bats,omitempty" cbor:"37,keyasint,omitempty"` ExtraFs map[string]*FsStats `json:"efs,omitempty" cbor:"21,keyasint,omitempty"` GPUData map[string]GPUData `json:"g,omitempty" cbor:"22,keyasint,omitempty"` // LoadAvg1 float64 `json:"l1,omitempty" cbor:"23,keyasint,omitempty"` @@ -43,7 +44,7 @@ type Stats struct { MaxBandwidth [2]uint64 `json:"bm,omitzero" cbor:"-"` // [sent bytes, recv bytes] // TODO: remove other load fields in future release in favor of load avg array LoadAvg [3]float64 `json:"la,omitempty" cbor:"28,keyasint"` - Battery [2]uint8 `json:"bat,omitzero" cbor:"29,keyasint,omitzero"` // [percent, charge state, current] + Battery [2]uint8 `json:"bat,omitzero" cbor:"29,keyasint,omitzero"` // [percent, charge state] NetworkInterfaces map[string][4]uint64 `json:"ni,omitempty" cbor:"31,keyasint,omitempty"` // [upload bytes, download bytes, total upload, total download] DiskIO [2]uint64 `json:"dio,omitzero" cbor:"32,keyasint,omitzero"` // [read bytes, write bytes] MaxDiskIO [2]uint64 `json:"diom,omitzero" cbor:"-"` // [max read bytes, max write bytes] diff --git a/internal/entities/system/system_test.go b/internal/entities/system/system_test.go new file mode 100644 index 00000000..c389a806 --- /dev/null +++ b/internal/entities/system/system_test.go @@ -0,0 +1,37 @@ +package system + +import ( + "encoding/json" + "testing" + + "github.com/fxamacker/cbor/v2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestStatsBatteryTransport(t *testing.T) { + stats := Stats{Battery: [2]uint8{0, 1}, Batteries: map[string]uint8{"Primary": 0, "Mouse": 75}} + + jsonData, err := json.Marshal(stats) + require.NoError(t, err) + var jsonPayload map[string]any + require.NoError(t, json.Unmarshal(jsonData, &jsonPayload)) + assert.Equal(t, []any{float64(0), float64(1)}, jsonPayload["bat"]) + assert.Equal(t, map[string]any{"Primary": float64(0), "Mouse": float64(75)}, jsonPayload["bats"]) + + cborData, err := cbor.Marshal(stats) + require.NoError(t, err) + var decoded Stats + require.NoError(t, cbor.Unmarshal(cborData, &decoded)) + assert.Equal(t, stats.Battery, decoded.Battery) + assert.Equal(t, stats.Batteries, decoded.Batteries) +} + +func TestStatsLegacyBatteryPayload(t *testing.T) { + data, err := json.Marshal(Stats{Battery: [2]uint8{50, 4}}) + require.NoError(t, err) + var payload map[string]any + require.NoError(t, json.Unmarshal(data, &payload)) + assert.Contains(t, payload, "bat") + assert.NotContains(t, payload, "bats") +} diff --git a/internal/records/records.go b/internal/records/records.go index ae986893..8e8dd2e1 100644 --- a/internal/records/records.go +++ b/internal/records/records.go @@ -186,6 +186,9 @@ func AverageSystemStatsSlice(records []system.Stats) system.Stats { // necessary because uint8 is not big enough for the sum batterySum := 0 + batteryCount := 0 + batterySums := make(map[string]uint64) + batteryCounts := make(map[string]uint64) // accumulate per-core usage across records var cpuCoresSums []uint64 // accumulate cpu breakdown [user, system, iowait, steal, idle] @@ -232,8 +235,15 @@ func AverageSystemStatsSlice(records []system.Stats) system.Stats { for i := range stats.DiskIoStats { sum.DiskIoStats[i] += stats.DiskIoStats[i] } - batterySum += int(stats.Battery[0]) - sum.Battery[1] = stats.Battery[1] + if hasBattery(stats.Battery, stats.Batteries) { + batterySum += int(stats.Battery[0]) + batteryCount++ + sum.Battery[1] = stats.Battery[1] + } + for name, percent := range stats.Batteries { + batterySums[name] += uint64(percent) + batteryCounts[name]++ + } // accumulate per-core usage if present if stats.CpuCoresUsage != nil { @@ -379,7 +389,15 @@ func AverageSystemStatsSlice(records []system.Stats) system.Stats { sum.LoadAvg[2] = twoDecimals(sum.LoadAvg[2] / count) sum.Bandwidth[0] = sum.Bandwidth[0] / uint64(count) sum.Bandwidth[1] = sum.Bandwidth[1] / uint64(count) - sum.Battery[0] = uint8(batterySum / int(count)) + if batteryCount > 0 { + sum.Battery[0] = uint8(batterySum / batteryCount) + } + if len(batterySums) > 0 { + sum.Batteries = make(map[string]uint8, len(batterySums)) + for name, total := range batterySums { + sum.Batteries[name] = uint8(total / batteryCounts[name]) + } + } // Average network interfaces if sum.NetworkInterfaces != nil { @@ -467,6 +485,10 @@ func AverageSystemStatsSlice(records []system.Stats) system.Stats { return sum } +func hasBattery(legacy [2]uint8, batteries map[string]uint8) bool { + return legacy != [2]uint8{} || len(batteries) > 0 +} + // Calculate the average stats of a list of container_stats records func (rm *RecordManager) AverageContainerStats(db dbx.Builder, records RecordIds) []container.Stats { allStats := make([][]container.Stats, 0, len(records)) diff --git a/internal/records/records_averaging_test.go b/internal/records/records_averaging_test.go index 2ce77de8..c1750e56 100644 --- a/internal/records/records_averaging_test.go +++ b/internal/records/records_averaging_test.go @@ -602,6 +602,28 @@ func TestAverageSystemStatsSlice_BatteryLastChargeState(t *testing.T) { assert.Equal(t, uint8(0), result.Battery[1]) // last record's charge state } +func TestAverageSystemStatsSlice_BatteriesIndependentSamples(t *testing.T) { + input := []system.Stats{ + {Battery: [2]uint8{80, 4}, Batteries: map[string]uint8{"Primary": 80, "Mouse": 0}}, + {Battery: [2]uint8{60, 3}, Batteries: map[string]uint8{"Primary": 60}}, + {Battery: [2]uint8{30, 4}, Batteries: map[string]uint8{"Mouse": 40}}, + {}, + } + result := records.AverageSystemStatsSlice(input) + assert.Equal(t, map[string]uint8{"Primary": 70, "Mouse": 20}, result.Batteries) + assert.Equal(t, uint8(56), result.Battery[0], "representative battery excludes absent samples") + assert.Equal(t, uint8(4), result.Battery[1], "representative state comes from its latest sample") +} + +func TestAverageSystemStatsSlice_ZeroRepresentativeBattery(t *testing.T) { + result := records.AverageSystemStatsSlice([]system.Stats{ + {Battery: [2]uint8{0, 1}, Batteries: map[string]uint8{"Primary": 0}}, + {}, + }) + assert.Equal(t, [2]uint8{0, 1}, result.Battery) + assert.Equal(t, map[string]uint8{"Primary": 0}, result.Batteries) +} + func TestAverageSystemStatsSlice_ThreeRecordsRounding(t *testing.T) { input := []system.Stats{ {Cpu: 10.0, Mem: 8.0}, diff --git a/internal/site/src/components/routes/system.tsx b/internal/site/src/components/routes/system.tsx index accd0233..495814ed 100644 --- a/internal/site/src/components/routes/system.tsx +++ b/internal/site/src/components/routes/system.tsx @@ -125,7 +125,7 @@ export default memo(function SystemDetail({ id }: { id: string }) { - + {hasGpuPowerData && } @@ -191,7 +191,7 @@ export default memo(function SystemDetail({ id }: { id: string }) { - + {pageBottomExtraMargin > 0 &&
} diff --git a/internal/site/src/components/routes/system/charts/sensor-charts.tsx b/internal/site/src/components/routes/system/charts/sensor-charts.tsx index e9a8e999..804eae5d 100644 --- a/internal/site/src/components/routes/system/charts/sensor-charts.tsx +++ b/internal/site/src/components/routes/system/charts/sensor-charts.tsx @@ -3,7 +3,7 @@ import AreaChartDefault from "@/components/charts/area-chart" import { batteryStateTranslations } from "@/lib/i18n" import { $fanFilter, $temperatureFilter, $userSettings } from "@/lib/stores" import { cn, decimalString, formatTemperature, toFixedFloat } from "@/lib/utils" -import type { ChartData, SystemStatsRecord } from "@/types" +import type { ChartData, SystemRecord, SystemStatsRecord } from "@/types" import { ChartCard, FilterBar } from "../chart-card" import LineChartDefault from "@/components/charts/line-chart" import { useStore } from "@nanostores/react" @@ -14,18 +14,60 @@ export function BatteryChart({ grid, dataEmpty, maxValues, + system, }: { chartData: ChartData grid: boolean dataEmpty: boolean maxValues: boolean + system: SystemRecord }) { - const showBatteryChart = chartData.systemStats.at(-1)?.stats.bat + const batteryNames = useMemo(() => { + const names = new Set() + for (const record of chartData.systemStats) { + for (const name in record.stats?.bats ?? {}) { + names.add(name) + } + } + return [...names].sort() + }, [chartData.systemStats]) + const hasNamedBatteries = batteryNames.length > 0 + const showBatteryChart = hasNamedBatteries || chartData.systemStats.some((record) => record.stats?.bat) if (!showBatteryChart) { return null } + if (hasNamedBatteries) { + const dataPoints = batteryNames.map((name, index) => ({ + label: name, + dataKey: ({ stats }: SystemStatsRecord) => stats?.bats?.[name], + color: `hsl(${(index * 360 + 226) / batteryNames.length}, 65%, 52%)`, + })) + return ( + + `${val}%`} + contentFormatter={({ value }) => `${value}%`} + itemSorter={(a, b) => b.value - a.value} + /> + + ) + } + return ( /** battery percent and state */ bat?: [number, BatteryState] + /** battery percentages by device name */ + bats?: Record /** network interfaces [upload bytes, download bytes, total upload bytes, total download bytes] */ ni?: Record }