[caumeira]

Adds pump-head RGB canvas support for the Galahad II LCD, streamed as JPEG over report B — same protocol main already uses for the glass. (An earlier draft of this PR used H.264 over report C; that got reverted in the second commit, so the final diff doesn't touch that path.)

The brightness fix that was originally part of this PR has been split out into #3 so it can merge on its own - see the review discussion below.

Summary

• Drives the pump head's 12 independently-addressable LEDs (of 24 physical - the rest mirror) from the lighting engine's canvas frame.
• Verified on real hardware: pump colours track the canvas live (see review thread for details / video).

Test plan

• dotnet build succeeds
• Verified on real Galahad II LCD hardware: pump head RGB tracks canvas colour changes live

[geretti]

Hey, thanks a lot for this, and for putting it through real hardware. We don't own a Vision, so you're the only one who can actually test it, which is why I'd rather get this right with you than rework it on our side. Suite is green here (8591 tests), nice.

A few things I ran into reading through it:

The description doesn't match the code anymore. The title and body still talk about H.264 over the C report, but your second commit reverted that, so the final diff streams JPEG over B exactly like main does. What's left is the 0x04 brightness mode, the immediate apply, and the pump RGB. Could you update the title/body? Also curious: does plain main stream on your unit once you turn Nexus Control on for the device? The rows default to off, and I want to be sure the logo issue was the code and not that switch.

Things I'd want changed before merging:

1. The firmware-mode branch in Galahad2LcdLightingFrameWriter.cs:148 reuses the Trinity mode table. On the LCD firmware the bytes diverge from 7 onward (7 is Ticker Tape there, not Vortex; see lianli-shared/src/rgb.rs in sgtaziz/lian-li-linux). It's also unreachable from the UI, which only shows firmware modes for lianli-aio. I'd just drop it and keep canvas only, which is the part people actually use.
2. Where does the 0x14 per-LED B-report layout come from (the 61-byte length, 24-byte zero prefix, reversed order)? Neither lian-li-linux nor OpenRGB has it. A capture or a decompile is totally fine, I just want it cited in a comment so the next person doesn't have to guess. And does your video show canvas colours on the pump head?
3. 0x04 isn't Galahad-specific: it's the family-wide LcdSetting mode in the reference driver, used for HydroShift too. So ApplyBrightness in LianLiAioHandshake.cs can use it unconditionally and the constructor parameter can go.

Smaller stuff:

4. OwnsOpenRgbDevice in the LCD provider: OpenRGB never enumerates 0x7395, so it's dead, and it would claim a real Trinity's row if someone had both. Same for the RemoveAll extension in the composite.
5. The ?? _galahad2 fallbacks in CompositeLightingDeviceProvider.cs (397, 423) send LCD ids to a provider that doesn't own them.
6. EncodePumpPerLed allocates a 1024-byte packet every tick; SendPumpZoneLighting already reuses the hub's _report, worth doing the same.
7. JpegPanelHub raises StateChanged while holding _lock. The other hubs notify from their connection worker outside the lock, safer to match that.

Two process things: please add a line confirming you've read and agree to CLA.md (it's in CONTRIBUTING.md), and if you're up for it, split this into two PRs. The brightness fix is small and I'd merge it right away; the pump RGB can take its time.

Thanks again, really good to have someone with the hardware on this.

[caumeira]

Thanks for the thorough review — replied to everything below.

Title/description — updated (was stale from an earlier draft that used H.264/report C; that got reverted in the second commit, final diff streams JPEG over B exactly like main).

Does plain main stream once Nexus Control is on? Confirmed, yes. With the switch off it shows Lian Li's own boot logo (expected — Nexus hasn't claimed the glass); switching it on streams normally, no code changes needed. So the "black screen" I hit earlier was just the row defaulting to off, not a main bug. Sorry for the confusion.

Firmware-mode branch in Galahad2LcdLightingFrameWriter — dropped. Canvas only now; the class doc explains why (unreachable from the UI, and no verified byte table for this device's firmware modes).

0x14 per-LED layout citation — I don't have a clean one, and want to be upfront about that rather than make something up. I had Codex derive this and no capture trail survived. I checked lian-li-linux myself (lianli-shared/src/rgb.rs and hydroshift_lcd/rgb.rs): its AIO-LCD pump path only drives the same 0x83 zone command EncodeLighting already sends, over a 64-byte A-report — no per-LED B-report command at all there. So this layout doesn't trace back to a known source. What I can offer instead is a direct hardware check: canvas colours visibly track live on the pump head on my unit, recorded on video, matching exactly what this function sends. Documented that honestly in a comment (verified-by-observation, not sourced) rather than inventing a citation.

Also confirmed while I was in there: the pump ring is 24 physical LEDs but only 12 are independently addressable (the other 12 mirror them) — checked against SignalRGB too, so PumpLedCount = 12 is correct, not a bug.

0x04 brightness mode — agreed, made it unconditional (LcdSettingMode, family-wide), dropped the constructor parameter. Split out into #3 along with the rest of the brightness fix. Tests updated (the HydroShift test that asserted mode 0x01 for brightness was wrong per this and is now fixed).

OwnsOpenRgbDevice / RemoveAll in composite — removed the dead LCD-provider IOpenRgbDeviceOwner implementation, and fixed the OpenRGB-row-removal condition in CompositeLightingDeviceProvider to only fire on the real wired Trinity's connection, not the LCD's.

?? _galahad2 fallbacks (397, 423) — fixed; both now go straight to _galahad2Lcd since the ids they handle only ever come from that provider.

EncodePumpPerLed allocation — now writes into the hub's reused _report buffer instead of allocating 1024 bytes every tick, matching SendPumpZoneLighting.

StateChanged under _lock — moved the notification outside the lock in Attach/Detach/Dispose, matching the other hubs' pattern.

CLA — read CONTRIBUTING.md / CLA.md, I've read and agree to the CLA.

Splitting into two PRs — done: #3 has just the brightness fix (should merge quickly), this PR is now pump-RGB only. Once #3 lands I'll rebase this branch on main so the diff here shrinks to just the pump RGB commits.

[caumeira]

Closing per your suggestion — this'll get resubmitted as a fresh, independent PR just for the pump RGB once #3 (the minimal brightness fix, which covers part of what this PR had) is settled. Thanks again for the review.

[geretti]

Thanks for the quick turnaround, and for being straight about where the 0x14 layout came from. That's the right call and it makes the next step clear.

Two things remain, one structural and one about that packet.

One Galahad provider, not two. Looking at it again, there's no reason for Galahad2LcdLightingDeviceProvider / Galahad2LcdLightingFrameWriter to exist. The wired Trinity and the LCD variant share the same pump command: lian-li-linux's has_pump_rgb covers Galahad2Lcd and Galahad2Vision with the same set_pump_light (0x83, inner/outer/both), and FanControl.LianLi and OpenRGB do the same. The only real difference is which object holds the HID handle. So please fold this into the existing Galahad2LightingDeviceProvider and Galahad2LightingFrameWriter: give them a small pump-transport interface (IsConnected + SendLighting(...)) that Galahad2Hub implements and the LCD's JpegPanelHub implements too, and delete the two new classes. That's how every other family in the app is done, one provider per family. It also removes the nullable provider from the composite: _galahad2Lcd?.IsConnected ?? false and the _galahad2Lcd! calls can't ship, and with one provider they simply go away.

The 0x14 per-LED report. Without a source it can't go in, even with the video, because three public drivers all say this pump is driven by 0x83 only. Two ways forward:

1. Ship the LCD pump on the reference-backed 0x83 two-ring path now. Folding into the existing provider gives you that for free (inner + outer colours from the canvas, same as the Trinity). Verify it on your unit and this PR is mergeable.
2. Per-LED as a follow-up PR once there is something to cite. You mentioned SignalRGB exposes 12 LEDs on this pump: its plugin is a plain .js in the SignalRGB install directory and has the actual packet layout, so quoting the relevant lines would be a proper reference. A USBPcap of SignalRGB or L-Connect driving it works too. One more thing worth checking on your side before that: send a canvas that is half red, half blue. If the pump head shows two colours, 0x14 is per-LED; if it shows one, the firmware is only reading the first triplet and the 0x83 bootstrap is what you're seeing on the video.

Small one: keep comments to a single line stating the fact. The provenance paragraph in Galahad2Protocol.cs and the class doc on the writer would be a one-liner each, and a comment shouldn't mention how it was derived.

#3 looks right. It changes the HydroShift brightness packet as well (0x01 to 0x04), so we'll run it on our HydroShift before merging; I'll update there.