Add a global AppIdService to the application architecture#231
Conversation
77a6abc to
2d562db
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a single, app-global Application Identifier service that is persisted via PreferencesService, and wires CaDA, MouldKing, and JIESTAR to use this shared AppId source (instead of vendor-specific persistence/logic).
Changes:
- Added
IAppIdentifierService+AppIdentifierServicethat generates/persists a random app identifier and can serve variable-length prefixes. - Refactored CaDA and JIESTAR device managers to pull their AppID from the shared service.
- Updated MouldKing architecture so devices can receive an app-wide AppID (via
IMouldKingDeviceManager) and patch telegram bytes [1] and [2]; updated/added tests accordingly.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| BrickController2/BrickController2/UI/Services/AppIdentifier/IAppIdentifierService.cs | New interface for requesting an AppID prefix of a given length. |
| BrickController2/BrickController2/UI/Services/AppIdentifier/AppIdentifierService.cs | New global AppID implementation backed by IPreferencesService. |
| BrickController2/BrickController2/UI/DI/UiModule.cs | Registers AppIdentifierService as a singleton in UI DI. |
| BrickController2/BrickController2/DeviceManagement/CaDA/CaDADeviceManager.cs | Switches CaDA to use the shared AppID and removes vendor-specific persistence. |
| BrickController2/BrickController2/DeviceManagement/JieStar/JieStarDeviceManager.cs | Switches JIESTAR to use the shared AppID and removes vendor-specific persistence. |
| BrickController2/BrickController2/DeviceManagement/MouldKing/IMouldKingDeviceManager.cs | New interface to expose the MouldKing AppID to devices. |
| BrickController2/BrickController2/DeviceManagement/MouldKing/MouldKingDeviceManager.cs | Uses shared AppID and exposes it via IMouldKingDeviceManager. |
| BrickController2/BrickController2/DeviceManagement/MouldKing/MouldKing.cs | Registers MouldKing device manager as IMouldKingDeviceManager. |
| BrickController2/BrickController2/DeviceManagement/MouldKing/MKBaseNibble.cs | Patches appId bytes into connect/base telegrams via IMouldKingDeviceManager. |
| BrickController2/BrickController2/DeviceManagement/MouldKing/MKBaseByte.cs | Patches appId bytes into connect/base telegrams via IMouldKingDeviceManager. |
| BrickController2/BrickController2/DeviceManagement/MouldKing/MK3_8.cs | Updates ctor signature to pass IMouldKingDeviceManager to base. |
| BrickController2/BrickController2/DeviceManagement/MouldKing/MK4.cs | Updates ctor signature to pass IMouldKingDeviceManager to base. |
| BrickController2/BrickController2/DeviceManagement/MouldKing/MK5.cs | Updates ctor signature to pass IMouldKingDeviceManager to base. |
| BrickController2/BrickController2/DeviceManagement/MouldKing/MK6.cs | Updates ctor signature to pass IMouldKingDeviceManager to base. |
| BrickController2/BrickController2.Tests/DeviceManagement/CaDA/CaDADeviceManagerTests.cs | Updates tests to use shared AppID preferences key + service. |
| BrickController2/BrickController2.Tests/DeviceManagement/MouldKing/MouldKingDeviceManagerTests.cs | Updates tests for new DI requirements and adds AppID assertion. |
| BrickController2/BrickController2.Tests/DeviceManagement/JieStar/JieStarDeviceManagerTests.cs | New test verifying JIESTAR uses the shared AppID. |
| BrickController2/BrickController2.Tests/DeviceManagement/DI/VendorBuilderTests.cs | Extends DI tests to register new required manager interfaces for device construction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
| { | ||
| public interface IAppIdentifierService | ||
| { | ||
| ReadOnlyMemory<byte> GetAppId(int length); |
There was a problem hiding this comment.
Thing to discuss whether we need to have additional 3 similar types: IJieStarDeviceManager, ICaDADeviceManager and IMouldKingDeviceManager just to define GetAppId() but having typical usage like this one:
appId: _cadaManager.GetAppId().Span[..2]
How about using this IAppIdentifierService to simply return 4 / 8 bytes as unique AppId and let all the consumers to use subset as they used it now.
WDYT?
As I discovered while reverse‑engineering the JIESTAR devices, the bytes at positions [1, 2] in their datagrams represent an ApplicationIdentifier.
Since JIESTAR and MouldKing devices share the same datagram structure, MouldKing also uses these two bytes as an ApplicationIdentifier.
With this PR, I introduced a global AppIdService, which generates a random ApplicationID on first use and persists it via the PreferencesService.
CaDA, MouldKing, and JIESTAR now all rely on this shared AppIdService.