-
Notifications
You must be signed in to change notification settings - Fork 0
[ADR-62] Separate the application into a cross-platform core application and a Windows-specific host #30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This project contains all the application logic, services, and HTML/JS UI that can be hosted for any platform. The AdaptiveRemote project is now a Windows-specific host application. All Windows-specific support (WPF, System.Speech) lives in this project.
…nit tests for all the cross-platform components of the application (pretty much all of them). The project itself is now cross-platform as well. AdaptiveRemote.Speech.Tests is a new test project that includes the GrammarTests, because those require the Windows System.Speech components to full test speech recognition end-to-end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request separates AdaptiveRemote into a cross-platform core library (AdaptiveRemote.App) and a Windows-specific host application (AdaptiveRemote). The main changes include:
- Creating a platform-independent AdaptiveRemote.App assembly with application logic, services, and HTML-based UI
- Moving Windows-specific functionality (System.Speech) to the AdaptiveRemote host project
- Introducing abstractions (IGrammar, ISpeechRecognitionEngine) to allow the Conversation subsystem to remain platform-independent
- Moving test project from net8.0-windows to net8.0 target framework
- Adding comprehensive test utilities and test files for various services
Key Changes
- Created abstraction layer for speech recognition components via IGrammar and wrapper classes
- Migrated test project to target net8.0 (cross-platform) instead of net8.0-windows
- Added extensive test utilities (MockLogger, MockFileSystem, MockExtensions, etc.) to support testing
- Added test coverage for multiple service layers (TiVo, Broadlink, Conversation, Lifecycle, etc.)
Reviewed changes
Copilot reviewed 42 out of 242 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/AdaptiveRemote.App.Tests/AdaptiveRemote.App.Tests.csproj | Changed from net8.0-windows to net8.0, updated project reference to AdaptiveRemote.App |
| test/AdaptiveRemote.App.Tests/GlobalAttributes.cs | Added SupportedOSPlatform attributes for windows and linux |
| test/AdaptiveRemote.App.Tests/GlobalUsings.cs | Added global usings for test utilities |
| test/AdaptiveRemote.App.Tests/TestUtilities/*.cs | Added comprehensive test utility classes for mocking and assertions |
| test/AdaptiveRemote.App.Tests/Services/**/*.cs | Added extensive test coverage for services across multiple subsystems |
| src/AdaptiveRemote/Services/Conversation/StaticGrammarProvider.cs | Introduced GrammarWrapper to wrap System.Speech.Grammar in IGrammar interface |
| src/AdaptiveRemote/Services/Conversation/SpeechRecognitionEngineWrapper.cs | Updated to work with IGrammar abstraction instead of concrete Grammar type |
| src/AdaptiveRemote/MainWindow.xaml | Updated namespace reference to include assembly qualifier for cross-assembly component access |
| src/AdaptiveRemote.App/wwwroot/css/theme.less | Added theme import file |
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
Test Results272 tests ±0 272 ✅ ±0 27s ⏱️ +16s Results for commit 37f2b72. ± Comparison against base commit 0637eb3. This pull request removes 58 and adds 57 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
|
The test pass needs to include the new test assembly, that’s probably why we lost test cases |
jodavis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make requested changes
src/AdaptiveRemote.App/Services/Conversation/SamplesRecorder.cs
Outdated
Show resolved
Hide resolved
src/AdaptiveRemote.App/Services/Lifecycle/AcceleratedServices.cs
Outdated
Show resolved
Hide resolved
src/AdaptiveRemote/Services/Conversation/StaticGrammarProvider.cs
Outdated
Show resolved
Hide resolved
test/AdaptiveRemote.App.Tests/Services/Conversation/GrammarTests.cs
Outdated
Show resolved
Hide resolved
…for configuring the app, not running it. I think it was only responsible for running so that AdaptiveRemote and AdaptiveRemote.Console could use the same loop, but now AdaptiveRemote.Console runs the same startup code in App that AdaptiveRemote does. The Electron host will not use this, so it's host-specific.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 45 out of 243 changed files in this pull request and generated no new comments.
Creates a platform-independent assembly (AdaptiveRemote.App) which includes all the application logic, services, and HTML-based UI. This DLL is hosted by the Windows-specific AdaptiveRemote project, which implements the WPF-based host application. It also provides System.Speech components for speech synthesis and recognition, through an abstraction that allows most of the Conversation subsystem to remain in the core App.