Add unit tests for CaDA RaceCar rev1 and rev2 datagram encoding

[J0EK3R]

I've added unit tests for the CaDARaceCar device for both revisions.

To make them pass, I had to apply two small fixes:

J0EK3R@5e4e0c6

The first fix is a simple call to InitDevice inside the CaDARaceCar constructor to have the instance initialized inside the UnitTests:

    public CaDARaceCar(string name, string address, byte[] deviceData, IDeviceRepository deviceRepository, IBluetoothLEService bleService, IMessageEncoderFactory messageEncoderFactory)
      : base(name, address, deviceData, deviceRepository, bleService)
    {
        // create message encoder for this device based on advertised data
        _messageEncoder = messageEncoderFactory.Create(deviceData);

        InitDevice();
    }

The second fix initializes the OutputValuesGroup with an invalid value:

    public void Initialize()
    {
        lock (_outputLock)
        {
            // reset all values
            _outputValues.AsSpan().Clear();
            _commitedOutputValues.AsSpan().Fill(TValue.One + TValue.One); // set to invalid value
            _values.AsSpan().Clear();
            // enable sending for the first round
            _sendAttemptsLeft = MAX_SEND_ATTEMPTS;
        }
    }

Without these changes, the previous implementation failed to detect output changes when all three channels were set to 1 before calling TryGetTelegram.

[vicocz]

This is not proper unit test as you are using already tested MessageEncoderFactory. The way you have composed this setup is more integration test than the unit test.

TEsts for TryGetTelegram method should just test that encoder is properly called and the mothed itself returns its mocked value.

[Copilot]

Pull request overview

Adds deterministic unit coverage for CaDA RaceCar datagram encoding (Rev1 + Rev2) and adjusts device initialization/output-change detection so the encoder can be exercised reliably from tests.

Changes:

• Add Rev1 and Rev2 datagram encoding unit tests for CaDARaceCar.
• Initialize CaDARaceCar in its constructor and widen TryGetTelegram visibility for test access.
• Adjust OutputValuesGroup.Initialize() to force initial “changed” detection.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
BrickController2/BrickController2/DeviceManagement/IO/OutputValuesGroup.cs	Tweaks initialization of committed outputs to affect first-change detection behavior.
BrickController2/BrickController2/DeviceManagement/CaDA/CaDARaceCar.cs	Ensures device/encoder init occurs for unit tests; adjusts TryGetTelegram accessibility.
BrickController2/BrickController2.Tests/DeviceManagement/CaDA/CaDARaceCarRev2DatagramTests.cs	Adds Rev2 connect/command datagram payload tests and instance-isolation checks.
BrickController2/BrickController2.Tests/DeviceManagement/CaDA/CaDARaceCarRev1DatagramTests.cs	Adds Rev1 connect/command datagram payload tests and instance-isolation checks.
BrickController2/BrickController2.Tests/DeviceManagement/CaDA/CaDADatagramTestsBase.cs	Introduces shared test base with mocked CaDA services and deterministic random/app id.

Suppressed comments (4)

BrickController2/BrickController2.Tests/DeviceManagement/CaDA/CaDARaceCarRev2DatagramTests.cs:209

• This test mutates the expectedPayload array that is supplied via [InlineData]. Attribute-provided arrays may be reused between test cases, so mutating them can cause cross-test interference and flaky results.

Clone expectedPayload before patching in the device address/footer.

        deviceAddress.CopyTo(expectedPayload, 3); // Copy device address to expectedPayload at index 3
        PayloadCommandFooter.CopyTo(expectedPayload, 12); // Copy footer to expectedPayload at index 12)

BrickController2/BrickController2.Tests/DeviceManagement/CaDA/CaDARaceCarRev1DatagramTests.cs:195

• This test mutates the expectedPayload array that is supplied via [InlineData]. Attribute-provided arrays may be reused between test cases, so mutating them can cause cross-test interference and flaky results.

Clone expectedPayload before patching in the device address.

        deviceAddress.CopyTo(expectedPayload, 2); // Copy device address to expectedPayload at index 2

BrickController2/BrickController2.Tests/DeviceManagement/CaDA/CaDARaceCarRev2DatagramTests.cs:29

• ToLower() is culture-sensitive; even though this is hex, using the invariant form avoids any locale-dependent behavior in test inputs.

        CaDARaceCar device = new CaDARaceCar("CaDARaceCar", BitConverter.ToString(deviceAddress).ToLower(), ScanData, _deviceRepository.Object, _bluetoothLEService.Object, _messageEncoderFactory);

BrickController2/BrickController2.Tests/DeviceManagement/CaDA/CaDARaceCarRev1DatagramTests.cs:28

• ToLower() is culture-sensitive; even though this is hex, using the invariant form avoids any locale-dependent behavior in test inputs.

        CaDARaceCar device = new CaDARaceCar("CaDARaceCar", BitConverter.ToString(deviceAddress).ToLower(), ScanData, _deviceRepository.Object, _bluetoothLEService.Object, _messageEncoderFactory);

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.