From 6448de73763ef01d98de2569940056465414934f Mon Sep 17 00:00:00 2001 From: RaRaiRen <248741121+RaRaiRen@users.noreply.github.com> Date: Fri, 6 Nov 2026 22:56:30 -0500 Subject: [PATCH] [ryoku] hub: key identical SMBus lighting devices by slot OpenRGB reports no serial for SMBus controllers, so every identical RAM stick got the bare-name key and shared one lighting.json entry. findLive then resolved that key to the first stick only: Hand back, set, follow and save all reached one stick and left the rest on the last frame Ryoku pushed. ENE DRAM modes cannot be saved and a warm reboot keeps the controller powered, so the stuck stick survived reboots too. An I2C location now keys the device by adapter name and address. The /dev/i2c-N node is dropped because it follows driver load order. The legacy bare-name entry is copied to every stick it stood for on the next scan and dropped once all have claimed it, and identical names sort by key so the cards keep their order. Note: Fixed: two identical RGB RAM sticks show as separate lighting devices, and handing one back no longer leaves the other stuck --- ryoku/hub/CHANGELOG.md | 6 ++ ryoku/hub/backend/lighting.go | 94 +++++++++++++++++++++++----- ryoku/hub/backend/lighting_test.go | 99 ++++++++++++++++++++++++++++++ 3 files changed, 184 insertions(+), 15 deletions(-) diff --git a/ryoku/hub/CHANGELOG.md b/ryoku/hub/CHANGELOG.md index 3f5cb12cdd..f529bac15b 100644 --- a/ryoku/hub/CHANGELOG.md +++ b/ryoku/hub/CHANGELOG.md @@ -3,6 +3,12 @@ ## Unreleased ### Fixed +- **Identical RAM sticks are separate lighting devices.** SMBus devices report + no serial, so every matching stick shared one entry and Hand back restored + only the first. Each stick is now keyed by its adapter and address (the + `/dev/i2c-N` number is ignored, since it moves between boots), the old shared + entry is copied to every stick on the next scan, and identical sticks list in + a stable order (`backend/lighting.go`). - Shortcut names, hints, categories and "not on" reasons follow the interface language in Keybinds and in Kit. The shortcut catalogue now enters the translation catalog, and the Keybinds page translates what it shows diff --git a/ryoku/hub/backend/lighting.go b/ryoku/hub/backend/lighting.go index 775e208fed..0e97c090c4 100644 --- a/ryoku/hub/backend/lighting.go +++ b/ryoku/hub/backend/lighting.go @@ -137,16 +137,63 @@ func saveLighting(s lightingState) error { } // deviceKey: an identity that survives a reboot. The USB path and the SDK index -// both move, so the name plus the serial is the stable pair. Two identical -// devices with no serial share one entry, which is honest: nothing about them -// distinguishes one from the other between sessions. +// both move, so the name plus the serial is the stable pair. An SMBus device +// (RAM sticks, some boards and graphics cards) reports no serial, but its +// adapter and address are fixed by the slot it sits in, so that pair tells +// identical sticks apart. Two identical devices with neither share one entry, +// which is honest: nothing about them distinguishes one from the other between +// sessions. func deviceKey(d orgbDevice) string { if d.Serial != "" { return d.Name + "#" + d.Serial } + if slot, ok := smbusSlot(d.Location); ok { + return d.Name + "#" + slot + } return d.Name } +var ( + smbusLocation = regexp.MustCompile(`^I2C: (.*), address (0x[0-9A-Fa-f]+)$`) + // the bus number follows driver load order, so it can move between boots + // while the adapter name and the address stay put. + smbusBusNode = regexp.MustCompile(`\s*\(?/dev/i2c-\d+\)?`) +) + +// smbusSlot reads "smbus:@
" out of an OpenRGB I2C location. +func smbusSlot(location string) (string, bool) { + m := smbusLocation.FindStringSubmatch(location) + if m == nil { + return "", false + } + adapter := strings.TrimSpace(smbusBusNode.ReplaceAllString(m[1], "")) + return "smbus:" + adapter + "@" + strings.ToLower(m[2]), true +} + +// legacyKeys: the keys this device's settings may still be stored under. An +// SMBus device was once keyed by its bare name, shared by every identical stick. +func legacyKeys(d orgbDevice) []string { + keys := d.Aliases + if d.Serial == "" { + if _, ok := smbusSlot(d.Location); ok { + keys = append(append([]string(nil), keys...), d.Name) + } + } + return keys +} + +func cloneSettings(s *lightingSettings) *lightingSettings { + c := *s + c.Colors = append([]string(nil), s.Colors...) + if s.ZoneColors != nil { + c.ZoneColors = make(map[string]string, len(s.ZoneColors)) + for z, hex := range s.ZoneColors { + c.ZoneColors[z] = hex + } + } + return &c +} + // ── the wire shape the Hub renders ────────────────────────────────────────── // lightMode = one mode reduced to the knobs it actually offers, so the UI can @@ -321,28 +368,32 @@ func lightingScan() lightReport { func rememberFirstSight(live []orgbDevice) lightingState { st := loadLighting() dirty := false + // a legacy entry is dropped only after every device it stood for has taken + // its own copy, so identical sticks that shared one all keep the look. + claimed := map[string]bool{} for _, d := range live { key := deviceKey(d) s := st.Devices[key] - for _, alias := range d.Aliases { + for _, alias := range legacyKeys(d) { old := st.Devices[alias] - if old == nil { + if old == nil || alias == key { continue } if s == nil || firstSightDefaults(s) { - old.Name = d.Name - st.Devices[key] = old - s = old + moved := cloneSettings(old) + moved.Name = d.Name + st.Devices[key] = moved + s = moved if d.Provider == auraProvider { - if old.Effect != "" { - old.Mode = auraModeForEffect(old.Effect, d) - old.Effect = "" + if moved.Effect != "" { + moved.Mode = auraModeForEffect(moved.Effect, d) + moved.Effect = "" } else { - old.Mode = auraModeForLegacyMode(old.Mode, d) + moved.Mode = auraModeForLegacyMode(moved.Mode, d) } } } - delete(st.Devices, alias) + claimed[alias] = true dirty = true } if s == nil { @@ -355,6 +406,11 @@ func rememberFirstSight(live []orgbDevice) lightingState { dirty = true } } + for alias := range claimed { + if !liveKey(live, alias) { + delete(st.Devices, alias) + } + } if dirty { _ = saveLighting(st) } @@ -482,13 +538,16 @@ func savedView(key string, s *lightingSettings) lightDevice { } // sortDevices: online devices first, then by name, so the list a user sees does -// not reshuffle between scans. +// not reshuffle between scans. Identical sticks fall back to their key. func sortDevices(ds []lightDevice) { sort.SliceStable(ds, func(i, j int) bool { if ds[i].Online != ds[j].Online { return ds[i].Online } - return ds[i].Name < ds[j].Name + if ds[i].Name != ds[j].Name { + return ds[i].Name < ds[j].Name + } + return ds[i].Key < ds[j].Key }) } @@ -1023,6 +1082,11 @@ func findLive(live []orgbDevice, key string) (orgbDevice, bool) { return orgbDevice{}, false } +func liveKey(live []orgbDevice, key string) bool { + _, ok := findLive(live, key) + return ok +} + // ── applying one device ───────────────────────────────────────────────────── func applyOne(o *orgbConn, live []orgbDevice, key string, s *lightingSettings, accent string) error { diff --git a/ryoku/hub/backend/lighting_test.go b/ryoku/hub/backend/lighting_test.go index 05dedf5b4c..5d7b965cd1 100644 --- a/ryoku/hub/backend/lighting_test.go +++ b/ryoku/hub/backend/lighting_test.go @@ -3,6 +3,7 @@ package main import ( "encoding/json" "errors" + "fmt" "io" "os" "path/filepath" @@ -289,6 +290,104 @@ func TestDeviceKeyIgnoresThePort(t *testing.T) { } } +func dramStick(address uint32, bus int, slot string) orgbDevice { + return orgbDevice{ + Name: "ENE DRAM", Vendor: "ENE", Type: "DRAM", Address: address, + Location: fmt.Sprintf("I2C: SMBus PIIX4 adapter port 0 at 0b00 (/dev/i2c-%d), address %s", bus, slot), + Modes: []orgbMode{ + {Name: "Direct", Value: 0, Flags: modeHasPerLEDColor}, + {Name: "Breathing", Value: 2, Flags: modeHasSpeed | modeHasPerLEDColor}, + {Name: "Rainbow", Value: 5, Flags: modeHasSpeed}, + }, + } +} + +func TestDeviceKeySeparatesIdenticalSMBusSticks(t *testing.T) { + first, second := dramStick(0, 8, "0x71"), dramStick(1, 8, "0x73") + if deviceKey(first) == deviceKey(second) { + t.Fatalf("two sticks share %q", deviceKey(first)) + } + if moved := dramStick(0, 9, "0x71"); deviceKey(moved) != deviceKey(first) { + t.Fatalf("a renumbered i2c bus moved the stick from %q to %q", deviceKey(first), deviceKey(moved)) + } + if got, want := deviceKey(first), "ENE DRAM#smbus:SMBus PIIX4 adapter port 0 at 0b00@0x71"; got != want { + t.Fatalf("key = %q, want %q", got, want) + } + if got := deviceKey(orgbDevice{Name: "Pad", Location: "HID: /dev/hidraw4"}); got != "Pad" { + t.Fatalf("a serial-less USB device keyed as %q, want its bare name", got) + } +} + +func TestFirstSightGivesEverySharedStickItsOwnCopy(t *testing.T) { + setupFollowTest(t) + if err := saveLighting(lightingState{Enabled: true, Devices: map[string]*lightingSettings{ + "ENE DRAM": { + Name: "ENE DRAM", Managed: true, Mode: "Breathing", Effect: "pulse", Source: "accent", + Colors: []string{"#E79389"}, Brightness: -1, Speed: 40, Restore: "Rainbow", + }, + }}); err != nil { + t.Fatal(err) + } + first, second := dramStick(0, 8, "0x71"), dramStick(1, 8, "0x73") + + st := rememberFirstSight([]orgbDevice{first, second}) + if _, ok := st.Devices["ENE DRAM"]; ok { + t.Fatal("the shared entry outlived the migration") + } + a, b := st.Devices[deviceKey(first)], st.Devices[deviceKey(second)] + for _, s := range []*lightingSettings{a, b} { + if s == nil || !s.Managed || s.Mode != "Breathing" || s.Effect != "pulse" || + s.Speed != 40 || s.Restore != "Rainbow" || len(s.Colors) != 1 { + t.Fatalf("stick settings = %+v, want the shared look", s) + } + } + a.Colors[0] = "#000000" + if b.Colors[0] != "#E79389" { + t.Fatal("the sticks still share one settings value") + } + if saved := loadLighting(); len(saved.Devices) != 2 { + t.Fatalf("saved devices = %d, want 2", len(saved.Devices)) + } +} + +func TestReleaseRestoresOnlyTheChosenStick(t *testing.T) { + setupFollowTest(t) + first, second := dramStick(4, 8, "0x71"), dramStick(7, 8, "0x73") + managed := func() *lightingSettings { + return &lightingSettings{Name: "ENE DRAM", Managed: true, Mode: "Breathing", + Source: "accent", Brightness: -1, Speed: -1, Restore: "Rainbow"} + } + if err := saveLighting(lightingState{Enabled: true, Devices: map[string]*lightingSettings{ + deviceKey(first): managed(), deviceKey(second): managed(), + }}); err != nil { + t.Fatal(err) + } + + oldAccess := lightingAccess + t.Cleanup(func() { lightingAccess = oldAccess }) + packet := captureWrite(t, 5, func(conn *orgbConn) error { + lightingAccess = func(_ bool, _ bool, _ lightingProviders, fn func(*orgbConn, []orgbDevice) error) error { + return fn(conn, []orgbDevice{first, second}) + } + captureStdout(t, func() { + if err := lightingRelease(deviceKey(second)); err != nil { + t.Fatal(err) + } + }) + return nil + }) + if packet.id != pktUpdateMode || packet.device != second.Address { + t.Fatalf("restore went to device %d (packet %d), want %d", packet.device, packet.id, second.Address) + } + st := loadLighting() + if st.Devices[deviceKey(second)].Managed { + t.Fatal("the released stick is still managed") + } + if !st.Devices[deviceKey(first)].Managed { + t.Fatal("releasing one stick released the other") + } +} + func TestStateDefaultsToOffAndSurvivesGarbage(t *testing.T) { dir := t.TempDir() t.Setenv("XDG_CONFIG_HOME", dir)