Skip to content

CaDARacecar: channels to separately control the front light and the rear light - #273

Open
J0EK3R wants to merge 16 commits into
vicocz:defaultfrom
J0EK3R:merge/vicocz/cada_lights
Open

J0EK3R wants to merge 16 commits into
vicocz:defaultfrom
J0EK3R:merge/vicocz/cada_lights

Conversation

@J0EK3R

@J0EK3R J0EK3R commented Sep 20, 2026

Copy link
Copy Markdown

I've added an additional channel in CaDARacecar.
Now it's possible to separately control the front light and the rear light.

grafik grafik

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

A stale assertion fails, while light threshold behavior, concurrent updates, and the malformed image asset need correction.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity · 2 Medium severity · 1 Low severity

Open (4)
What changed in this PR

Adds independent front and rear light controls to CaDA race cars.

Changes:

  • Expands CaDA race cars from three to four channels.
  • Encodes both lights as a shared bitfield.
  • Adds separate light selectors and icons.
File Description
UI/​Images/​rc_light_front_on_off.png Adds the front-light icon.
UI/​Controls/​DeviceChannelSelector.xaml.cs Wires the fourth channel selector.
UI/​Controls/​DeviceChannelSelector.xaml Adds separate front/rear light rows.
Protocols/​CaDAProtocol.cs Removes obsolete flag mapping.
DeviceManagement/​CaDA/​RaceCarMessageEncoderRev2.cs Sends the light bitfield.
DeviceManagement/​CaDA/​RaceCarMessageEncoder.cs Sends the light bitfield for legacy cars.
DeviceManagement/​CaDA/​CaDARaceCar.cs Combines two light channels into bit flags.
BrickController2.Tests/​DeviceManagement/​CaDA/​RaceCarMessageEncoderTests.cs Updates encoder expectations.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread BrickController2/BrickController2/DeviceManagement/CaDA/CaDARaceCar.cs Outdated
Comment thread BrickController2/BrickController2/DeviceManagement/CaDA/CaDARaceCar.cs Outdated
Comment thread BrickController2/BrickController2/DeviceManagement/CaDA/RaceCarMessageEncoder.cs Outdated
Comment thread BrickController2/BrickController2/UI/Controls/DeviceChannelSelector.xaml Outdated
@J0EK3R
J0EK3R force-pushed the merge/vicocz/cada_lights branch 3 times, most recently from 9e414d4 to c7df53a Compare September 22, 2026 05:25
@vicocz
vicocz requested a balanced review from Copilot September 23, 2026 20:04

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The front-light PNG contains substantial invalid trailing data, and key channel-bitfield behavior lacks coverage.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (4)
Previously missed (1)

In code that hasn't changed since last review

Low severity Add Rev2 tests for rear-only and both-light bitfields

BrickController2/​BrickController2/​DeviceManagement/​CaDA/​RaceCarMessageEncoderRev2.cs:97

The updated Rev2 encoder is only tested with light values 0 and 1, which both produced the same result with the removed MapAsFlag. Add cases for rear-only (0x02) and both lights (0x03) so this protocol revision's new bitfield behavior is actually verified.

Comment thread BrickController2/BrickController2/DeviceManagement/CaDA/CaDARaceCar.cs Outdated
@J0EK3R
J0EK3R force-pushed the merge/vicocz/cada_lights branch from 993b408 to 444813b Compare September 27, 2026 12:55
@J0EK3R
J0EK3R requested a review from vicocz September 27, 2026 17:26
@vicocz
vicocz requested a balanced review from Copilot September 29, 2026 16:50
@vicocz vicocz added this to the 2026.2 milestone Sep 29, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Half-precision bitwise operations corrupt the combined light value, and negative light inputs no longer preserve prior behavior.

Review effort: Balanced
Findings: 1 High severity · 2 Medium severity

Open (3)
Resolved since last review (1)

Comment thread BrickController2/BrickController2/DeviceManagement/IO/OutputValuesGroup.cs Outdated
Comment thread BrickController2/BrickController2/DeviceManagement/CaDA/CaDARaceCar.cs Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Negative light inputs regress existing behavior, and the Rev2 rear-light encoding lacks coverage.

Review effort: Balanced
Findings: 2 Medium severity

Open (2)
Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Low severity Add Rev2 rear-light bit and checksum coverage

BrickController2/​BrickController2/​DeviceManagement/​CaDA/​RaceCarMessageEncoderRev2.cs:97

The existing Rev2 tests only pass light values 0 and 1, so they would also pass with the old MapAsFlag implementation and do not exercise this PR's rear-light bit (0x02) or combined state (0x03). Add a Rev2 encoder case for 0x02 or 0x03 that verifies byte 9 and the resulting checksum.

@J0EK3R

J0EK3R commented Sep 30, 2026 •

Copy link
Copy Markdown
Author

Now CaDARaceCarChannelSelectorView looks like this

Another idea: we could change color of the the rear light icon to red...

grafik

J0EK3R added 9 commits September 30, 2026 06:30
SetLights now sets the bit if abs(value) > 0.5, otherwise clears it. Updated RaceCarMessageEncoderTests to encode (Half)0b00000011 and revised expected bytes with inline comments.
Replaced _lightsBitArray and SetLights with direct bit manipulation using the new SetOutputBit method in OutputValuesGroup. SetOutputBit enables atomic setting/clearing of individual bits for numeric TValue types, simplifying light state management and removing redundant fields and methods.
Refactored light output bit setting for channels 2 and 3 in CaDARaceCar to use ExecuteLocked with a function, improving thread safety and encapsulation. Introduced static SetLights method for setting/clearing bits. Replaced SetOutputBit in OutputValuesGroup with ExecuteLocked for flexible value modification.
J0EK3R and others added 6 commits September 30, 2026 06:30
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Reworked the XAML Grid to use inline RowDefinitions="*,Auto,Auto,*" and ColumnDefinitions="*,Auto, Auto, *" and removed nested inner Grid wrappers. Flattened child elements so Image and ColorImage controls are direct Grid children using Grid.Row, Grid.Column and Grid.RowSpan/Grid.ColumnSpan. Consolidated two lightbulb icons into a single ColorImage spanning columns 1–2 at row 2 and replaced the second lightbulb with an "online_prediction" icon spanning columns 1–2. ChannelSelectorRadioButton controls remain in column 3 for rows 0–3 but were moved into the flattened Grid. Result: simpler, more compact XAML with adjusted layout behavior via column/row spans.
Added targeted unit tests to verify light-flag encoding and final 16-byte payloads for RaceCar message encoders.

- RaceCarMessageEncoderRev2Tests.cs
  - Added three [Theory] tests:
    EncodeValues_WithZeroValuesAndFrontLightOn,
    EncodeValues_WithZeroValuesAndRearLightOn,
    EncodeValues_WithZeroValuesAndFrontLightAndRearLightOn.
  - Parameterized with InlineData (deviceId, appId, light bits, expected v1/v2/v4/sequence).
  - Tests create encoder via Create(deviceId, appId, sequence) and call EncodeValues([Zero, Zero, (Half)value]).
  - Assert the produced payload is exactly 16 bytes and matches the expected sequence:
    header 0xBB, 0x11, 0x11; deviceId (low, high); appId (low, high); v1, v2, light-flag byte (0x01 front, 0x10 rear, 0x11 both), v4; sequence; tail 0xCC, 0xB8, 0x92, 0xB0.
  - Notes that sequence is not incremented when speed and steering are zero.
  - Uses existing Create helper to pass deviceId/appId as little-endian byte pairs.

- RaceCarMessageEncoderTests.cs
  - Added two [Fact] tests:
    Encode_WithFrontLightValue_EncodesAndEncryptsValues and
    Encode_WithRearLightValue_EncodesAndEncryptsValues.
  - Each constructs an encoder, calls Encode([Zero, Zero, (Half)0b00000001]), asserts 16-byte length and that the encoded result ends with the expected channel/tail bytes (verifying encrypted channel data related to light flags).

These tests ensure correct placement of front/rear light bits in the control byte, exact payload structure, and that encryption/channel bytes reflect the light-flag state.
@J0EK3R
J0EK3R force-pushed the merge/vicocz/cada_lights branch from 3d116b1 to c66f260 Compare September 30, 2026 04:40
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.

3 participants