CADA RaceCar Rev2 [WIP]

[vicocz]

No description provided.

[Copilot]

Pull request overview

This PR introduces support for a second revision of the CaDA RaceCar device (CaDA_RaceCar_Rev2) and refactors the CaDA device registration to use the new vendor pattern. The PR is marked as Work in Progress (WIP), which aligns with the investigation nature indicated in the title.

Changes:

• Added CaDA_RaceCar_Rev2 device type and implementation with improved output value management using OutputValuesGroup
• Refactored CaDA device registration from module-based to vendor-based pattern using the new Vendor infrastructure
• Fixed multiple spelling errors ("advertisment" → "advertisement", "persistant" → "persistent")

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
DeviceType.cs	Added CaDA_RaceCar_Rev2 enum value
DeviceTypeToSmallImageConverter.cs	Added Rev2 case mapping to use same image as Rev1
DeviceTypeToImageConverter.cs	Added Rev2 case mapping to use same image as Rev1
OutputValuesGroup.cs	Changed SetOutput return type from void to bool to indicate if value changed
DeviceManagementModule.cs	Removed CaDA device registrations (moved to vendor module)
ICaDAPlatformService.cs	Added overload for TryGetRfPayload with manufacturerId parameter
ICaDADeviceManager.cs	New interface extending device manager interfaces with AppId property
CaDa.cs	New vendor class for CaDA device registration
CaDARaceCarRev2.cs	New device implementation for hardware revision 2
CaDADeviceManager.cs	Added AppId property, Rev2 detection, and spelling corrections
CaDAPlatformService.cs (iOS)	Implemented new TryGetRfPayload overload with session data
CaDAPlatformService.cs (WinUI)	Implemented new TryGetRfPayload overload with pass-through logic
CaDAPlatformService.cs (Android)	Implemented new TryGetRfPayload overload with pass-through logic
BleScanner.cs	Fixed spelling errors and removed unused variable
AdvertisementExtensions.cs	Fixed class name spelling and added NonConnectableUndirected support
MockSetups.cs	Added test helper for iOS platform service mocking
CaDARaceCarRev2Tests.cs	Comprehensive test coverage for Rev2 device telegram generation
CaDADeviceManagerTests.cs	Added tests for Rev2 device detection and AppId property

Comments suppressed due to low confidence (1)

BrickController2/BrickController2.WinUI/Extensions/AdvertisementExtensions.cs:14

• Adding NonConnectableUndirected advertisement type support is a functional change that allows the scanner to process advertisements from devices that use this type. The comment "some per advertisement devices which are not directly connectable" could be clearer. Consider rephrasing to "supports devices that advertise but are not directly connectable" or "supports non-connectable advertising devices."

---

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

[Copilot]

The device registration in CaDa.cs uses ContainerBuilder.RegisterDevice with explicit DeviceType parameters, which is inconsistent with the vendor pattern used by other vendors like Lego and MouldKing. The CaDARaceCar and CaDARaceCarRev2 classes should implement IDeviceType interface with static Type and TypeName properties, and then use builder.RegisterDevice() without explicit DeviceType parameters. This would make the registration consistent with other vendors in the codebase.

Suggested change

	builder.ContainerBuilder.RegisterDevice<CaDARaceCar>(DeviceType.CaDA_RaceCar);
	builder.ContainerBuilder.RegisterDevice<CaDARaceCarRev2>(DeviceType.CaDA_RaceCar_Rev2);
	builder.ContainerBuilder.RegisterDevice<CaDARaceCar>();
	builder.ContainerBuilder.RegisterDevice<CaDARaceCarRev2>();

[Copilot]

Pull request overview

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

---

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

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

BrickController2/BrickController2.WinUI/Extensions/AdvertisementExtensions.cs:14

• Comment grammar: "some per advertisement devices" is unclear/incorrect English. Consider rewording to something like "some advertising-only devices" or "some devices advertise as non-connectable" so the intent is unambiguous.

---

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

[Copilot]

DeviceType.CaDA_RaceCar_Rev2 was added, but the UI channel selector still only enables the CaDA section for DeviceType.CaDA_RaceCar (see UI/Controls/DeviceChannelSelector.xaml(.cs)), and its radio buttons are hard-coded to DeviceType="CaDA_RaceCar". As a result, Rev2 devices likely won't show the correct channel mapping UI. Please update the selector visibility logic and the CaDA radio buttons to also support CaDA_RaceCar_Rev2 (or refactor to share the same section for both).

Suggested change

	CaDA_RaceCar_Rev2,
	CaDA_RaceCar_Rev2 = CaDA_RaceCar,

[Copilot]

The default TryGetRfPayloadRev2 implementation always allocates a new array via rawData.ToArray(). In the hot path (CaDARaceCarRev2.TryGetTelegram) the payload is already a byte[], so on non-iOS platforms this introduces an avoidable extra allocation per update. Consider changing the Rev2 overload to accept byte[] (like TryGetRfPayload) or otherwise avoid copying when the caller already provides an array (and only allocate on platforms that need to reformat the payload, e.g. iOS).

Suggested change

	bool TryGetRfPayloadRev2(ReadOnlySpan<byte> rawData, out byte[] rfPayload)
	{
	// default implementation: copy raw data
	rfPayload = rawData.ToArray();
	bool TryGetRfPayloadRev2(byte[] rawData, out byte[] rfPayload)
	{
	// default implementation: use raw data directly
	rfPayload = rawData;

[Copilot]

This constructor validates deviceData length against a hard-coded 16, but the expected Rev2 payload size is already defined as ICaDAPlatformService.DefaultPayloadRev2Length. Using the shared constant here (and in other Rev2 length checks) would avoid future mismatches if the protocol size ever changes.

Suggested change

	if ((deviceData?.Length) != 16)
	if ((deviceData?.Length) != ICaDAPlatformService.DefaultPayloadRev2Length)