Skip to content

feat(lighting): Galahad II LCD pump RGB (two-ring, shared provider) - #4

Closed
caumeira wants to merge 1 commit into
hello-nexus:mainfrom
caumeira:fix/galahad2-lcd-pump-rgb
Closed

caumeira wants to merge 1 commit into
hello-nexus:mainfrom
caumeira:fix/galahad2-lcd-pump-rgb

Conversation

@caumeira

Copy link
Copy Markdown
Contributor

Resubmit of the pump RGB half of the earlier PR (#1, closed), addressing the review there.

Summary

  • One Galahad2LightingDeviceProvider/Galahad2LightingFrameWriter now drives both the wired Trinity and the LCD variant, through a small IGalahad2PumpTransport (IsConnected + SendLighting) that Galahad2Hub and the LCD's JpegPanelHub each implement - only which object holds the HID handle differs. No second provider, no nullable fallbacks in the composite.
  • Pump RGB rides the same 0x83 inner/outer/both command the wired Trinity already uses - confirmed against lian-li-linux, OpenRGB, and FanControl.LianLi, all of which describe this pump as 0x83-only.
  • Dropped the earlier per-LED (0x14) path entirely: it had no citable source, so it's not shipped uncited. Can come back as a follow-up once there's something to cite (lian-li-linux doesn't have it; checked).
  • LightingDevicesCatalog marks the Galahad II LCD row rgb-capable (previously screen-only).

Test plan

  • dotnet build succeeds
  • Verified on real Galahad II LCD hardware: canvas colours apply to the pump head through the two-ring path

I've read and agree to the CLA (CONTRIBUTING.md / CLA.md).

One Galahad2LightingDeviceProvider/FrameWriter drives both the wired
Trinity and the LCD variant - the reference driver's has_pump_rgb
covers Galahad2Lcd and Galahad2Vision with the same set_pump_light
(0x83, inner/outer/both), and only which object holds the HID handle
differs. IGalahad2PumpTransport (IsConnected + SendLighting) abstracts
that; Galahad2Hub and the LCD's JpegPanelHub each implement it, and
the provider uses whichever is actually connected. This removes the
nullable second provider from the composite along with its `?? `
fallbacks.

No per-LED (0x14) path: three public drivers (lian-li-linux, OpenRGB,
FanControl.LianLi) all describe this pump as 0x83-only, and the
per-LED byte layout had no citable source, so it's dropped rather than
shipped uncited. The LCD pump now gets the same inner/outer canvas
colours as the Trinity through the shared 0x83 path.

LightingDevicesCatalog marks the row rgb-capable (previously
screen-only).

Tested on real Galahad II LCD hardware: canvas colours apply to the
pump head through the two-ring path.
@caumeira

Copy link
Copy Markdown
Contributor Author

Both points addressed:

One provider — folded the LCD into the existing Galahad2LightingDeviceProvider/Galahad2LightingFrameWriter. IGalahad2PumpTransport (IsConnected + SendLighting) is implemented by Galahad2Hub and by the LCD's JpegPanelHub; the provider picks whichever is connected (ActiveTransport). Deleted Galahad2LcdLightingDeviceProvider/Galahad2LcdLightingFrameWriter, and the _galahad2Lcd?.IsConnected ?? false / _galahad2Lcd! fallbacks in the composite are gone with them - there's nothing left to fall back from.

0x14 — took option 1: dropped it, shipped the LCD pump on the reference-backed 0x83 two-ring path instead (same command the wired Trinity already uses). Verified on my unit: canvas inner/outer colours apply to the pump head. Didn't end up needing the half-red/half-blue check since per-LED isn't in this PR anymore - if it comes back later I'll run that first and cite whatever it points to (SignalRGB's plugin source sounds like the right place to start).

Comments trimmed to one line each, no provenance narrative.

Also closed #1 and opened this fresh off main, per your note.

@caumeira

Copy link
Copy Markdown
Contributor Author

Reopened as #5 with the structure fixed (the LCD's pump is one 12-LED ring, not the Trinity's two 1-LED rings this version assumed) and the 0x14 per-LED command restored with a real citation. GitHub wouldn't let me reopen this PR directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant