From 662e00ad8721fa77b98fa88556b5631141ce9181 Mon Sep 17 00:00:00 2001 From: Stefanie Jane Date: Thu, 10 Sep 2026 13:15:22 -0700 Subject: [PATCH] fix(lianli): restore wireless RGB immediately after PWM upkeep Fan-speed packets can interrupt direct RGB playback and expose the receiver's onboard rainbow effect. Re-send the latest frame after the complete PWM batch, before clock and pairing traffic delay recovery. Keep a reusable cache limited to the discovered topology, retain black frames from pause and stop, and clear it on initialization. Share the normal RGB encoder to preserve cluster routing and packet pacing. Verify latest-frame recovery, black frames, reversed fan chains, input normalization, and connection reset without changing output cadence. Co-Authored-By: Nova (GPT-6) --- .../src/drivers/lianli/wireless/mod.rs | 156 +++++++++++------- .../tests/lianli_wireless_tests.rs | 131 ++++++++++++++- .../80-lian-li-tl-lcd-wireless-driver.md | 12 ++ 3 files changed, 236 insertions(+), 63 deletions(-) diff --git a/crates/hypercolor-hal/src/drivers/lianli/wireless/mod.rs b/crates/hypercolor-hal/src/drivers/lianli/wireless/mod.rs index 24add828a..3e297e618 100644 --- a/crates/hypercolor-hal/src/drivers/lianli/wireless/mod.rs +++ b/crates/hypercolor-hal/src/drivers/lianli/wireless/mod.rs @@ -113,6 +113,9 @@ struct WirelessState { topology_frozen: bool, streaming_started: bool, clock_sent: bool, + /// The last submitted pixels, including black frames, retained across idle upkeep. + /// None means no lighting frame has arrived in this session. + latest_colors: Option>, } impl WirelessState { @@ -415,6 +418,75 @@ impl WirelessControllerProtocol { commands.push(Self::tx_command(stream_prep_packet(index, channel), false)); } } + + /// Encode live delivery and PWM recovery through the same per-cluster wire path. + fn push_rgb_frame( + state: &WirelessState, + colors: &[[u8; 3]], + frame_number: Option, + buffer: &mut CommandBuffer<'_>, + ) { + let master = Self::master_mac(state); + let mut offset = 0_usize; + for cluster in &state.table.clusters { + let leds_per_fan = usize::from(cluster.model.leds_per_fan()); + let led_count = usize::from(cluster.fan_count) * leds_per_fan; + let mut raw = Vec::with_capacity(led_count * 3); + for led in 0..led_count { + let color = colors.get(offset + led).copied().unwrap_or([0, 0, 0]); + raw.extend_from_slice(&color); + } + offset += led_count; + if cluster.right_attach { + raw = reverse_fan_order(&raw, leds_per_fan, usize::from(cluster.fan_count)); + } + + let transfer = rgb_transfer( + cluster.mac, + master, + effect_index_for(&raw), + u8::try_from(led_count).unwrap_or(u8::MAX), + LIVE_TOTAL_FRAMES, + LIVE_INTERVAL_MS, + &raw, + ); + if let Some(frame_number) = frame_number + && frame_number.is_multiple_of(FRAME_TRACE_EVERY) + { + trace!( + frame_number, + mac = %format_mac(cluster.mac), + led_count, + data_envelopes = transfer.data.len(), + first_pixel = ?raw.get(..3), + "wireless frame encoded" + ); + } + for repeat in 0..HEADER_REPEATS { + let tail_delay = if repeat + 1 < HEADER_REPEATS { + HEADER_REPEAT_GAP + } else { + SLICE_PACING + }; + Self::push_envelope( + buffer, + &transfer.header, + cluster.channel, + cluster.rx_type, + tail_delay, + ); + } + for envelope in &transfer.data { + Self::push_envelope( + buffer, + envelope, + cluster.channel, + cluster.rx_type, + SLICE_PACING, + ); + } + } + } } impl Protocol for WirelessControllerProtocol { @@ -478,7 +550,17 @@ impl Protocol for WirelessControllerProtocol { fn encode_frame_into(&self, colors: &[[u8; 3]], commands: &mut Vec) { let mut state = self.state.write().unwrap_or_else(PoisonError::into_inner); state.topology_frozen = true; - let master = Self::master_mac(&state); + let led_count = state + .table + .clusters + .iter() + .map(FanCluster::led_count) + .sum::(); + let led_count = usize::try_from(led_count).unwrap_or(0); + let latest_colors = state.latest_colors.get_or_insert_with(Vec::new); + latest_colors.clear(); + latest_colors.extend_from_slice(&colors[..colors.len().min(led_count)]); + latest_colors.resize(led_count, [0, 0, 0]); let mut buffer = CommandBuffer::new(commands); if !state.streaming_started { @@ -497,64 +579,7 @@ impl Protocol for WirelessControllerProtocol { } let frame_number = self.frames_encoded.fetch_add(1, Ordering::Relaxed); - let mut offset = 0_usize; - for cluster in &state.table.clusters { - let leds_per_fan = usize::from(cluster.model.leds_per_fan()); - let led_count = usize::from(cluster.fan_count) * leds_per_fan; - let mut raw = Vec::with_capacity(led_count * 3); - for led in 0..led_count { - let color = colors.get(offset + led).copied().unwrap_or([0, 0, 0]); - raw.extend_from_slice(&color); - } - offset += led_count; - if cluster.right_attach { - raw = reverse_fan_order(&raw, leds_per_fan, usize::from(cluster.fan_count)); - } - - let transfer = rgb_transfer( - cluster.mac, - master, - effect_index_for(&raw), - u8::try_from(led_count).unwrap_or(u8::MAX), - LIVE_TOTAL_FRAMES, - LIVE_INTERVAL_MS, - &raw, - ); - if frame_number.is_multiple_of(FRAME_TRACE_EVERY) { - trace!( - frame_number, - mac = %format_mac(cluster.mac), - led_count, - data_envelopes = transfer.data.len(), - first_pixel = ?raw.get(..3), - "wireless frame encoded" - ); - } - for repeat in 0..HEADER_REPEATS { - let tail_delay = if repeat + 1 < HEADER_REPEATS { - HEADER_REPEAT_GAP - } else { - SLICE_PACING - }; - Self::push_envelope( - &mut buffer, - &transfer.header, - cluster.channel, - cluster.rx_type, - tail_delay, - ); - } - for envelope in &transfer.data { - Self::push_envelope( - &mut buffer, - envelope, - cluster.channel, - cluster.rx_type, - SLICE_PACING, - ); - } - } - + Self::push_rgb_frame(&state, colors, Some(frame_number), &mut buffer); buffer.finish(); } @@ -566,7 +591,8 @@ impl Protocol for WirelessControllerProtocol { } /// The 1 Hz upkeep: refresh the table, hold every cluster at the PWM it - /// reported, and broadcast the clock. Streaming mode is armed here only + /// reported, restore the latest RGB frame, and broadcast the clock. + /// Streaming mode is armed here only /// when no frame has done it yet. fn keepalive_commands(&self) -> Vec { let mut state = self.state.write().unwrap_or_else(PoisonError::into_inner); @@ -592,6 +618,16 @@ impl Protocol for WirelessControllerProtocol { Self::envelope_commands(&mut commands, &envelope, cluster.channel, cluster.rx_type); } + // PWM traffic can interrupt the receiver's RGB playback. Restore the + // submitted frame before clock, pairing, or the next actor turn can delay it. + if let Some(colors) = &state.latest_colors { + let mut rgb_commands = Vec::new(); + let mut buffer = CommandBuffer::new(&mut rgb_commands); + Self::push_rgb_frame(&state, colors, None, &mut buffer); + buffer.finish(); + commands.extend(rgb_commands); + } + let payload = clock_payload(WallClock::now_local()); let envelope = clock_sync_envelope(master, &payload, !state.clock_sent); Self::envelope_commands(&mut commands, &envelope, channel, RF_BROADCAST_SLOT); diff --git a/crates/hypercolor-hal/tests/lianli_wireless_tests.rs b/crates/hypercolor-hal/tests/lianli_wireless_tests.rs index fed61552d..895c595c8 100644 --- a/crates/hypercolor-hal/tests/lianli_wireless_tests.rs +++ b/crates/hypercolor-hal/tests/lianli_wireless_tests.rs @@ -16,7 +16,7 @@ use hypercolor_hal::drivers::lianli::wireless::frame::{ clock_sync_envelope, pwm_envelope, reverse_fan_order, rgb_transfer, }; use hypercolor_hal::drivers::lianli::wireless::tinyuz; -use hypercolor_hal::protocol::{Protocol, ResponseTolerance, TransferType}; +use hypercolor_hal::protocol::{Protocol, ProtocolCommand, ResponseTolerance, TransferType}; use hypercolor_hal::registry::TransportType; use hypercolor_types::device::DeviceTopologyHint; @@ -513,9 +513,134 @@ fn upkeep_does_not_restart_the_stream_once_it_is_armed() { .all(|command| command.data[..4] != TX_VIDEO_START), "no video start after the first frame" ); - assert_eq!(commands.len(), 1 + 3 * 4, "poll, two PWM holds, one clock"); + assert_eq!( + commands.len(), + 1 + 3 * 4 + 2 * 3 * 4, + "poll, PWM, RGB, clock" + ); let again = protocol.keepalive_commands(); - assert_eq!(again.len(), 1 + 3 * 4); + assert_eq!(again.len(), commands.len()); +} + +/// All PWM envelopes precede recovery; unrelated upkeep must wait until the +/// full RGB transfer (including every data packet and repeated header) is sent. +fn assert_upkeep_restores_frame( + protocol: &WirelessControllerProtocol, + expected: &[ProtocolCommand], +) { + let commands = protocol.keepalive_commands(); + let pwm_end = 1 + protocol.clusters().len() * 4; + let rgb_end = pwm_end + expected.len(); + assert_eq!(commands.len(), rgb_end + 4, "poll, PWM, RGB, clock only"); + assert_eq!(commands[0].transfer_type, TransferType::Companion); + for pwm in commands[1..pwm_end].chunks_exact(4) { + assert_eq!(pwm[0].data[5], RfSubCommand::Pwm as u8); + } + for (actual, expected) in commands[pwm_end..rgb_end].iter().zip(expected) { + assert_eq!( + actual.data, expected.data, + "restore the complete latest frame" + ); + assert_eq!(actual.post_delay, expected.post_delay); + assert_eq!(actual.transfer_type, expected.transfer_type); + assert_eq!(actual.expects_response, expected.expects_response); + } + assert_eq!(commands[rgb_end].data[5], RfSubCommand::ClockSync as u8); +} + +#[test] +fn upkeep_restores_the_latest_frame_before_clock_and_retains_it_while_idle() { + let protocol = discovered_protocol(); + let _ = protocol.encode_frame(&[[255, 0, 0]; 5 * 26]); + let colors: Vec<_> = (0_u8..130) + .map(|index| [index, index.wrapping_mul(13), 255 - index]) + .collect(); + let latest = protocol.encode_frame(&colors); + assert_upkeep_restores_frame(&protocol, &latest); + // Idle delivery has no new encode calls, but PWM ticks still need recovery. + assert_upkeep_restores_frame(&protocol, &latest); + assert_eq!(protocol.capabilities().max_fps, 30); + assert_eq!(protocol.frame_interval(), Duration::from_millis(33)); +} + +#[test] +fn upkeep_restores_pause_black_and_empty_frames_without_resurrecting_old_colors() { + let protocol = discovered_protocol(); + let _ = protocol.encode_frame(&[[255, 0, 0]; 5 * 26]); + let black = protocol.encode_frame(&[[0, 0, 0]; 5 * 26]); + assert_upkeep_restores_frame(&protocol, &black); + + let _ = protocol.encode_frame(&[[0, 255, 0]; 5 * 26]); + let empty = protocol.encode_frame(&[]); + assert_eq!( + empty + .iter() + .map(|command| &command.data) + .collect::>(), + black + .iter() + .map(|command| &command.data) + .collect::>(), + "an empty frame pads all known fans with black" + ); + assert_upkeep_restores_frame(&protocol, &empty); +} + +#[test] +fn upkeep_preserves_right_attached_cluster_order_and_normalizes_input_lengths() { + let protocol = WirelessControllerProtocol::new(); + protocol + .parse_response(&captured_master_reply()) + .expect("master"); + protocol + .parse_response(&table_with(&[ + record([0x11; 6], 0, 12, 27), + record([0x22; 6], 0, 1, 27), + ])) + .expect("one right-attached cluster and one normal cluster"); + let _ = protocol.keepalive_commands(); + let colors: Vec<_> = (0_u8..100).map(|index| [index, 0, 0]).collect(); + let exact = protocol.encode_frame(&colors[..78]); + let excess = protocol.encode_frame(&colors); + assert_eq!( + exact + .iter() + .map(|command| &command.data) + .collect::>(), + excess + .iter() + .map(|command| &command.data) + .collect::>(), + "pixels outside the physical topology are ignored" + ); + assert_upkeep_restores_frame(&protocol, &excess); + let short = protocol.encode_frame(&colors[..10]); + assert_upkeep_restores_frame(&protocol, &short); +} + +#[test] +fn a_new_session_discards_cached_colors_before_rediscovery() { + let protocol = discovered_protocol(); + let _ = protocol.encode_frame(&[[255, 0, 0]; 5 * 26]); + let _ = protocol.init_sequence(); + protocol + .parse_response(&captured_master_reply()) + .expect("controller rediscovered"); + protocol + .parse_response(&table_with(&[record([0x33; 6], 0, 1, 27)])) + .expect("new cluster discovered"); + let commands = protocol.keepalive_commands(); + assert_eq!( + commands.len(), + 1 + 2 + 4 + 4, + "no cached RGB in a new session" + ); + assert_eq!(&commands[1].data[..4], &TX_VIDEO_START); + assert_eq!(commands[3].data[5], RfSubCommand::Pwm as u8); + assert_eq!(commands[7].data[5], RfSubCommand::ClockSync as u8); + + let fresh = protocol.encode_frame(&[[1, 2, 3]; 26]); + assert_upkeep_restores_frame(&protocol, &fresh); } #[test] diff --git a/docs/specs/80-lian-li-tl-lcd-wireless-driver.md b/docs/specs/80-lian-li-tl-lcd-wireless-driver.md index f4d662daf..56e181719 100644 --- a/docs/specs/80-lian-li-tl-lcd-wireless-driver.md +++ b/docs/specs/80-lian-li-tl-lcd-wireless-driver.md @@ -809,6 +809,18 @@ occasionally spikes RPM. and snapped to the newest frame each tick. The reference arms video mode once as well. +PWM writes can interrupt direct RGB output and briefly expose the receiver's +onboard rainbow effect. After writing the PWM batch, upkeep immediately +re-sends each cluster's latest RGB frame through the normal transfer codec, +before clock or pairing work. Recovery starts only after a frame has +been encoded, preserves black frames, and never re-arms streaming mode. +The cached frame belongs to the connection session and is cleared on init. +The upstream driver documents the same interruption in +[issue 83](https://github.com/sgtaziz/lian-li-linux/issues/83) and restores +cached direct colors in [PR 149](https://github.com/sgtaziz/lian-li-linux/pull/149). +Packet-level tests establish recovery ordering and payload fidelity; +physical flicker removal still requires observation on the affected fans. + This holds user-set speeds steady without making Hypercolor a fan-curve product. Exact clock-blob field values are validated on hardware before the descriptor ships enabled (ยง11). Both upkeep streams and the GetDev poll run