Conversation
9d61e20 to
c82f3e9
Compare
Test Results375 tests +63 373 ✅ +68 2m 32s ⏱️ + 1m 31s For more details on these failures, see this check. Results for commit a24c5fa. ± Comparison against base commit 9113844. This pull request removes 3 and adds 65 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
|
@copilot There's some flakiness in the end to end tests in this branch. There's a NullReferenceException coming from the LifecycleViewController, specifically in the anonymous callback method passed to LastOrDefault. That callback is very simple, the only thing that could be null is the item in the activities list, but nothing should be adding nulls to that list. There are locks around list changes so it shouldn't be a threading issue. Take a look at the code, run the tests, and try to get all the tests in the repo passing. Make sure the whole solution builds without warnings or errors when you are done. |
04d95c2 to
9d70cdf
Compare
|
@copilot rebase this branch on the main branch, resolving conflicts along the way. Each time you resolve conflicts, make sure the solution builds without warnings or errors and that all tests pass before continuing to rebase. |
… for coordinating accelerated services, host configuration, and test hooks consistently across hosts. Each host can override parts of the startup sequence as needed.
* ADR-149: Split ITestEndpoint interface and update startup flow * ADR-149: Add TestSpeechRecognitionEngine for test control Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com> * ADR-149: Update test infrastructure for new startup flow Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com> * ADR-149: Fix ITestServiceProvider to implement IDisposable and increase timeout Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com> * ADR-149: Address code review comments - refactor test infrastructure Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com> * Fix formatting warnings and verify Playwright installation Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com> * Add SpeechSteps for TestSpeechRecognition test Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com> * ADR-149: Implement speech test infrastructure (partial) Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com> * ADR-149: Add Builder.ConfigureTestServices and speech test infrastructure (WIP) Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com> * ADR-149: Complete speech recognition test - all E2E tests pass! ✅ Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com> * ADR-149: Major refactoring per code review - move test code, simplify steps Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com> * ADR-149: Simplify SpeechSteps, fix AcceleratedServices config parsing Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com> * ADR-149: Fix RegisterTestService to load contract type from correct assembly Playwright installed, tests now working - 3/4 pass Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com> * Some more clean-up to follow existing patterns. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com> Co-authored-by: Joe Davis <ElwoodMoves@hotmail.com>
- Flush the log file in dispose so that the full log gets written - Detect non-zero exit codes and log standard output/error to the test log when non-zero
…2, so that two processes don't end up using the same port.
…troller (#104) * Fix NullReferenceException race condition in LifecycleViewController.Activity.Dispose --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com>
* Implement ADR-141: Integration test for conversation modal UI * Fix conversation modal UI test to use speech commands * Reordered test steps and changed the speaking rate, which significantly increases the likelihood that the speech modal will be caught by the test. Ran repeatedly with no failures. * Remove the test for the ISpeechTestService, which was only there to ensure the test service was working before we consumed it in the new test. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com> Co-authored-by: Joe Davis <ElwoodMoves@hotmail.com>
* Implement ADR-142: Modal message service with FIFO queuing * Fix: propagate cancellationToken to body in ModalMessageService * Address PR review: extract ModalMessageUI, Singleton DI, synchronous tests, cancellation tests --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com>
…. We reserve the port earlier and hold onto it right up until the WebView2 is initializing, so there's less of a window for some other process to get the same port.
#102) * Override ISpeechSynthesis in tests, remove audio device detection, add speech verification steps * Fix null reference in WaitForSpokenPhrase; initialize to empty array --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com>
…ammed commands (#101) * Implement ADR-148: Move hardcoded IR payloads to ProgrammaticSettings * Rework per reviewer: IOptionsSnapshot<IRDataSettings>, IsCommandEnabled, config pipeline * Address reviewer feedback: AddIniFile, root config, INI section format, test improvements --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com>
* ADR-147: Add ProgramDelegate support to CommandServiceBase --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com>
478a694 to
d7e41c5
Compare
* ADR-146: Implement Programming Mode UI with Learn button (#105) * Implement ADR-146: Programming mode UI with Prog button and E2E test * Address review feedback: share button logic via CommandButtonBase, pass ProgramMode as prop from Root * ADR-144: IR learning protocol for BroadlinkCommandService + CI build fixes (#107) * ADR-144: Implement BroadlinkCommandService ProgramDelegate with IR learning protocol * ADR-144: Address code review - use Stopwatch for timing, poll before delay * Fix pre-existing CI build warnings: trailing whitespace and unused using directives * ADR-144: Address review feedback - remove timeout, handle null connection, update ExecuteAsync, improve tests * Implement ADR-145: Add E2E tests for programming mode with learning protocol support * Fix PersistSettings.ValueRegex bug and update scenario for isolation * Fix test failures related to new tests - Unit test failures in PayloadTests and PersistSettings due to behavior changes - "data-programmed" attribute was set or not set, rather than set to "true" or "false" as expected - LearnedDataResponsePayload needed a DataLength for correct protocol implementation - Created a new `IUIButtonTestObject` paradigm to separate finding a button and checking its properties, to improve error reporting - Programming mode feature now programs "Volume Down", which required a way to map that label to the correct "Down" button - Added automatic log verification to the end of every test, and included skipping to the last line of the log that was searched, in case logs have expected errors in them - Made PlaywrightUITestService initialization synchronous (does not return until Playwright is loaded) so that subsequent calls can fail more quickly. Previously they all had very long timeouts to give time for lazy initialization of Playwright. - Added WaitHelpers.WaitForState to encapsulate a common waiting pattern that emerged * ADR-145: Add E2E tests for programmable IR command learning mode * Changes from code review feedback * ADR-145: Address code review feedback - refactor learning mode and update tests - Fix ProgramButton.razor: IsProgrammed only applies when IsEnabled (has ProgramAsync) - Simplify LifecycleView to plain data object; rename ProgrammingCancellationToken → LearningCancellationToken - Add EnterLearningMode/ExitLearningModeAsync to ILifecycleViewController; implement in LifecycleViewController with CTS management - Refactor LifecycleCommandService: remove LifecycleView dependency, use ILifecycleViewController; Exit has ProgramHandler so it remains enabled in learning mode - Update Phrases: Broadlink_ProgrammingCommand uses markdown H1; add Broadlink_NotConnected - BroadlinkCommandService uses Phrases.Broadlink_NotConnected - Add LEARN button CSS (smaller font, margin) in layout.less and app.css - Fix SimulatedBroadlinkSteps: update comment, add malformed check to 'sent' step - Update BroadlinkDevice.feature: replace 3 packet checks with single 'sent' step - Update ProgrammingMode.feature: rename to Learning Mode UI, add programmed/not-programmed and contrast checks, add persistence test, update modal text to markdown H1 format * Improve the UI of unprogrammed buttons vs. programmed buttons. * Improve the UI of unprogrammed commands and the learn button * Disable accessibility checks in the BlazorWebViewUITestService, so that accessibility checks can be added in various application states without causing failures in WPF hosts. --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: jodavis <6740581+jodavis@users.noreply.github.com> Co-authored-by: Joe Davis <ElwoodMoves@hotmail.com>
…bView2 resources, to try to resolve occasional timing related startup issues with parallel runs of the application. (#122) Co-authored-by: Joe Davis <ElwoodMoves@hotmail.com>
* Serialize E2E tests --------- Co-authored-by: Joe Davis <ElwoodMoves@hotmail.com>
There was a problem hiding this comment.
Pull request overview
Implements ADR-127 “Programmable IR Commands” end-to-end: programmable/learning-mode UI, Broadlink IR learning + persistence via programmatic settings, and expanded unit/E2E test infrastructure to support deterministic speech + UI automation.
Changes:
- Added programming mode (Learn) + ProgramButton UI, with persisted IR payloads loaded from an INI-backed programmatic settings file.
- Implemented Broadlink learning protocol primitives (enter learning, poll learned data) and a modal message queue service used by conversation + programming flows.
- Refactored E2E host startup/testing services (test endpoint/service provider injection, Playwright helpers, new Gherkin scenarios).
Reviewed changes
Copilot reviewed 115 out of 115 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| test/_doc_EndToEndTests.md | Documents step-definition patterns (sync steps, polling, bindings). |
| test/AdaptiveRemote.EndtoEndTests.TestServices/WaitHelpers.cs | Adds WaitForState helper; refactors retry/polling helpers. |
| test/AdaptiveRemote.EndtoEndTests.TestServices/TestSpeechSynthesis.cs | Test speech synthesis that records phrases + simulates speaking delay. |
| test/AdaptiveRemote.EndtoEndTests.TestServices/TestSpeechRecognitionEngine.cs | Test speech recognition engine for programmatically raising recognition events. |
| test/AdaptiveRemote.EndtoEndTests.TestServices/SpeechTestService.cs | RPC-facing speech test service for raising recognition + inspecting spoken phrases. |
| test/AdaptiveRemote.EndtoEndTests.TestServices/SimulatedBroadlink/SimulatedBroadlinkDeviceBuilder.cs | Updates simulated Broadlink device builder API. |
| test/AdaptiveRemote.EndtoEndTests.TestServices/SimulatedBroadlink/SimulatedBroadlinkDevice.cs | Adds simulated learning mode behavior + 0x6A subcommand handling. |
| test/AdaptiveRemote.EndtoEndTests.TestServices/SimulatedBroadlink/ISimulatedBroadlinkDeviceExtensions.cs | Adds polling helper for simulated learning mode. |
| test/AdaptiveRemote.EndtoEndTests.TestServices/SimulatedBroadlink/ISimulatedBroadlinkDevice.cs | Expands simulated Broadlink device interface (learning mode + payload injection). |
| test/AdaptiveRemote.EndtoEndTests.TestServices/PlaywrightUITestService.cs | Refactors UI test service to return button objects + add text/css helpers. |
| test/AdaptiveRemote.EndtoEndTests.TestServices/PlaywrightButtonTestObject.cs | Introduces a button test object abstraction over Playwright locators. |
| test/AdaptiveRemote.EndtoEndTests.TestServices/IUITestServiceExtensions.cs | Adds richer synchronous UI helpers (modal waits, programmed state, etc.). |
| test/AdaptiveRemote.EndtoEndTests.TestServices/ITestEndpointExtensions.cs | Adds provider-based test service creation + service injection helper. |
| test/AdaptiveRemote.EndtoEndTests.TestServices/IApplicationTestServiceExtensions.cs | Adds polling helper for conversation listening state. |
| test/AdaptiveRemote.EndtoEndTests.TestServices/Host/SimulatedEnvironment.cs | Writes test-time programmatic settings INI and injects test speech services. |
| test/AdaptiveRemote.EndtoEndTests.TestServices/Host/ISimulatedEnvironment.cs | Exposes configured test IR payloads to steps. |
| test/AdaptiveRemote.EndtoEndTests.TestServices/Host/AdaptiveRemoteHost.cs | Adds speech test service accessor; removes stdout/stderr fields. |
| test/AdaptiveRemote.EndtoEndTests.TestServices/Host/AdaptiveRemoteHost.Builder.cs | Updates host startup flow (service injection hooks, WebView debugging). |
| test/AdaptiveRemote.EndtoEndTests.TestServices/BlazorWebViewUITestService.cs | Skips axe accessibility checks for WebView2. |
| test/AdaptiveRemote.EndtoEndTests.TestServices/AudioDetectionHelper.cs | Removes audio device detection helper (tests no longer require real audio). |
| test/AdaptiveRemote.EndtoEndTests.TestServices/ApplicationTestService.cs | Exposes conversation listening state by walking remote definition tree. |
| test/AdaptiveRemote.EndtoEndTests.Host.Console/Hooks/ConsoleHostTestHooks.cs | Removes audio device requirement for console host tests. |
| test/AdaptiveRemote.EndToEndTests.Steps/UISteps.cs | Updates UI steps for new wait helpers + modal/programmed assertions. |
| test/AdaptiveRemote.EndToEndTests.Steps/SpeechSteps.cs | Adds speech steps (“When I say…”) and listening/spoken assertions. |
| test/AdaptiveRemote.EndToEndTests.Steps/SimulatedBroadlinkSteps.cs | Adds learning-mode and payload matching steps; consolidates send-signal verification. |
| test/AdaptiveRemote.EndToEndTests.Steps/LogVerificationSteps.cs | Adds log incremental reading + “expect error” step + after-scenario check. |
| test/AdaptiveRemote.EndToEndTests.Steps/ISpeechTestServiceExtensions.cs | Adds test-side phrase→semantics parsing + “Speak” helpers. |
| test/AdaptiveRemote.EndToEndTests.Steps/Hooks/EnvironmentSetupHooks.cs | Clears Broadlink packets per scenario; orders host-stop hook. |
| test/AdaptiveRemote.EndToEndTests.Host.Wpf/Hooks/WpfHostTestHooks.cs | Removes audio device requirement for WPF host tests. |
| test/AdaptiveRemote.EndToEndTests.Host.Wpf/Features/Shared/ProgrammingMode.feature | New E2E scenarios for learning mode UI + programming flows. |
| test/AdaptiveRemote.EndToEndTests.Host.Wpf/Features/Shared/ProgrammableCommandSettings.feature | New E2E scenarios validating settings-backed command enablement/payloads. |
| test/AdaptiveRemote.EndToEndTests.Host.Wpf/Features/Shared/ConversationModalUI.feature | New E2E scenario verifying conversation modal behavior via test speech. |
| test/AdaptiveRemote.EndToEndTests.Host.Wpf/Features/Shared/BroadlinkDevice.feature | Updates Broadlink feature to use consolidated “sent signal” assertion. |
| test/AdaptiveRemote.App.Tests/TestUtilities/MockOptionsSnapshot.cs | Adds options snapshot test helper. |
| test/AdaptiveRemote.App.Tests/Services/ProgrammaticSettings/PersistSettingsTests.cs | Adds tests for INI section format write/read. |
| test/AdaptiveRemote.App.Tests/Services/ModalMessages/ModalMessageServiceTests.cs | Adds unit tests for modal message service behavior (queueing/cancel/fault). |
| test/AdaptiveRemote.App.Tests/Services/Conversation/ConversationControllerTests.cs | Updates tests for modal message service integration. |
| test/AdaptiveRemote.App.Tests/Services/CommandServiceBaseTests.cs | Adds unit tests for new programming delegate support. |
| test/AdaptiveRemote.App.Tests/Services/Broadlink/PayloadTests.cs | Adds tests for new learning payload types and learned data response parsing. |
| src/AdaptiveRemote/WpfAppHostRunner.cs | Adds WPF-specific AppHostRunner implementation. |
| src/AdaptiveRemote/Services/Lifecycle/WpfAcceleratedServices.cs | Switches WebView debugging config to boolean + updates visibility of service registration method. |
| src/AdaptiveRemote/Services/Lifecycle/BlazorWebViewDebugger.cs | Allocates remote-debugging port dynamically and configures WebView2 args. |
| src/AdaptiveRemote/MainWindow.xaml.cs | Disposes WebView on window close. |
| src/AdaptiveRemote/AppHostConfiguration.cs | Removes unused accelerated services host builder extension. |
| src/AdaptiveRemote/App.xaml.cs | Refactors startup to use WpfAppHostRunner. |
| src/AdaptiveRemote.Headless/StubSpeech.cs | Adds simulated speech delay in headless host. |
| src/AdaptiveRemote.Headless/Program.cs | Refactors headless host startup to use AppHostRunner. |
| src/AdaptiveRemote.Console/Program.cs | Updates console app to new runner-based startup. |
| src/AdaptiveRemote.App/wwwroot/css/layout.less | Adds LEARN button placement. |
| src/AdaptiveRemote.App/wwwroot/css/button_ui.less | Adds “not programmed” visual style + border adjustments. |
| src/AdaptiveRemote.App/wwwroot/css/app.min.css | Updates minified CSS output to include new styles/placement. |
| src/AdaptiveRemote.App/wwwroot/css/app.css | Updates compiled CSS output to include new styles/placement. |
| src/AdaptiveRemote.App/compilerconfig.json | Enables CSS minification for compiled output. |
| src/AdaptiveRemote.App/Utilities/Errors.cs | Adds learning timeout + program-was-shut-down errors. |
| src/AdaptiveRemote.App/Settings.sample.ini | Adds sample INI with IRData section and payloads. |
| src/AdaptiveRemote.App/Services/Testing/TestingSettings.cs | Replaces WebView port config with WebViewDebugging boolean. |
| src/AdaptiveRemote.App/Services/Testing/TestServiceProvider.cs | Adds post-build test service provider for creating RPC services in-scope. |
| src/AdaptiveRemote.App/Services/Testing/TestEndpointService.cs | Refactors test endpoint to support service injection + startup coordination. |
| src/AdaptiveRemote.App/Services/Testing/IUITestService.cs | Replaces button-level operations with FindButtonByLabelAsync + modal text helpers. |
| src/AdaptiveRemote.App/Services/Testing/IUIButtonTestObject.cs | Adds RPC contract for button interactions (visible/enabled/programmed/click). |
| src/AdaptiveRemote.App/Services/Testing/ITestServiceProvider.cs | Adds RPC contract for creating test services via provider. |
| src/AdaptiveRemote.App/Services/Testing/ITestEndpointHooks.cs | Adds host-side hooks for injection + providing services to tests. |
| src/AdaptiveRemote.App/Services/Testing/ITestEndpoint.cs | Updates test endpoint contract to injection/startup/provider model. |
| src/AdaptiveRemote.App/Services/Testing/ISpeechTestService.cs | Adds RPC contract for speech test control. |
| src/AdaptiveRemote.App/Services/Testing/IApplicationTestService.cs | Adds GetIsListeningAsync to test application state. |
| src/AdaptiveRemote.App/Services/Testing/DisabledTestEndpointHooks.cs | Adds no-op hooks when testing isn’t enabled. |
| src/AdaptiveRemote.App/Services/ProgrammaticSettings/_doc_ProgrammaticSettings.md | Documents programmatic settings file format. |
| src/AdaptiveRemote.App/Services/ProgrammaticSettings/PersistSettings.cs | Adds INI section parsing/writing and improves validation. |
| src/AdaptiveRemote.App/Services/ModalMessages/_doc_ModalMessages.md | Documents modal message service design/queueing. |
| src/AdaptiveRemote.App/Services/ModalMessages/ModalMessageService.cs | Adds channel-based FIFO modal message queue service. |
| src/AdaptiveRemote.App/Services/ModalMessages/IModalMessageService.cs | Adds modal message service contract. |
| src/AdaptiveRemote.App/Services/Lifecycle/LifecycleViewController.cs | Adds learning-mode state + cancellation token management. |
| src/AdaptiveRemote.App/Services/Lifecycle/LifecycleCommandService.cs | Adds Learn command behavior + program handler for exit/learn toggling. |
| src/AdaptiveRemote.App/Services/Lifecycle/AcceleratedServices.cs | Adds early parsing of command-line config + startup test endpoint init. |
| src/AdaptiveRemote.App/Services/ILifecycleViewController.cs | Adds learning-mode enter/exit API. |
| src/AdaptiveRemote.App/Services/Conversation/ConversationSettings.cs | Adjusts speaking rate default. |
| src/AdaptiveRemote.App/Services/Conversation/ConversationController.cs | Routes speech output through modal message service (keepAlive support). |
| src/AdaptiveRemote.App/Services/Commands/_spec_ProgrammableCommands.md | Expands programmable commands spec and test-host injection design. |
| src/AdaptiveRemote.App/Services/Commands/StaticCommandGroupProvider.cs | Removes hard-coded IR payloads; adds Learn command to gutter. |
| src/AdaptiveRemote.App/Services/CommandServiceBase.cs | Adds support for per-command programming delegate + lifecycle handling. |
| src/AdaptiveRemote.App/Services/Broadlink/_doc_BroadlinkProtocol.md | Adds documented protocol overview for IR learning. |
| src/AdaptiveRemote.App/Services/Broadlink/LearnedDataResponsePayload.cs | Adds payload parser for learned IR response data. |
| src/AdaptiveRemote.App/Services/Broadlink/IRDataSettings.cs | Adds settings type for configured IR payloads. |
| src/AdaptiveRemote.App/Services/Broadlink/IDeviceConnection.cs | Adds learning mode methods to device connection contract. |
| src/AdaptiveRemote.App/Services/Broadlink/EnterLearningPayload.cs | Adds payload type for “enter learning” command. |
| src/AdaptiveRemote.App/Services/Broadlink/DeviceConnection.cs | Implements learning commands (enter learning + check learned data). |
| src/AdaptiveRemote.App/Services/Broadlink/CheckLearnedDataPayload.cs | Adds payload type for “check learned data” command. |
| src/AdaptiveRemote.App/Services/Broadlink/BroadlinkSettings.cs | Adds learn polling interval setting and fixes typo. |
| src/AdaptiveRemote.App/Services/Broadlink/BroadlinkCommandService.cs | Loads IR payloads from settings; adds programming/learning workflow + persistence. |
| src/AdaptiveRemote.App/Models/Phrases.cs | Adds Broadlink programming phrases/errors. |
| src/AdaptiveRemote.App/Models/ModalMessageView.cs | Adds modal message view model. |
| src/AdaptiveRemote.App/Models/LifecycleView.cs | Adds programming-mode flag + learning cancellation token. |
| src/AdaptiveRemote.App/Models/IRCommand.cs | Removes hard-coded IR data from model. |
| src/AdaptiveRemote.App/Models/ConversationView.cs | Removes speaking message state (now handled by modal service). |
| src/AdaptiveRemote.App/Models/Command.cs | Adds ProgramAsync delegate property. |
| src/AdaptiveRemote.App/Logging/MessageLogger.cs | Adds logging for programming operations + test endpoint hooks + learning logs. |
| src/AdaptiveRemote.App/Logging/FileLoggerProvider.cs | Flushes writer on dispose. |
| src/AdaptiveRemote.App/Configuration/TestingHostBuilderExtensions.cs | Registers ITestServiceProvider when test control port is set. |
| src/AdaptiveRemote.App/Configuration/SettingsKeys.cs | Adds keys for programmatic settings + IRData section. |
| src/AdaptiveRemote.App/Configuration/HostBuilderExtensions.cs | Loads INI file into configuration and wires programmatic settings options. |
| src/AdaptiveRemote.App/Configuration/ConversationHostBuilderExtensions.cs | Registers modal message service and view model for conversation. |
| src/AdaptiveRemote.App/Configuration/BroadlinkHostBuilderExtensions.cs | Binds Broadlink + IRData settings and adjusts fake-mode check. |
| src/AdaptiveRemote.App/Components/Root.razor | Passes programming mode into Remote component. |
| src/AdaptiveRemote.App/Components/RemoteLayout.razor | Switches to ProgramButton when in program mode and passes flag recursively. |
| src/AdaptiveRemote.App/Components/Remote.razor | Adds modal message UI and forwards program mode. |
| src/AdaptiveRemote.App/Components/ProgramButton.razor | Adds program-mode button behavior + programmed/not-programmed styling hooks. |
| src/AdaptiveRemote.App/Components/ModalMessageUI.razor | New modal message overlay component. |
| src/AdaptiveRemote.App/Components/ConversationUI.razor | Removes speaking-message overlay (now centralized modal UI). |
| src/AdaptiveRemote.App/Components/CommandButtonBase.cs | Adds shared Blazor button base class with property change subscription. |
| src/AdaptiveRemote.App/Components/CommandButton.razor | Refactors to use new base class and proper disabled attribute. |
| src/AdaptiveRemote.App/AppHostRunner.cs | Adds shared startup runner with test-service injection hooks. |
| src/AdaptiveRemote.App/AdaptiveRemote.App.csproj | Adds INI config package and InternalsVisibleTo for E2E test services. |
| Directory.Packages.props | Adds Microsoft.Extensions.Configuration.Ini package version. |
| .github/workflows/build-and-test.yml | Splits unit vs E2E runs and publishes E2E logs as artifacts. |
You can also share your feedback on Copilot code review. Take the survey.
| public CancellationToken EnterLearningMode() | ||
| { | ||
| CancellationTokenSource cts = new(); | ||
| _learningCts = cts; | ||
| ViewModel.LearningCancellationToken = cts.Token; | ||
| ViewModel.IsProgrammingMode = true; | ||
| return cts.Token; | ||
| } |
There was a problem hiding this comment.
We should keep in mind that the device is already in learning mode when the cancellation occurs, so starting the next learning cycle may cause problems if a second learn attempt is started during the first one.
Perhaps the better option would be to prevent a second operation from starting if there is already one in progress.
There was a problem hiding this comment.
Go with the option that entering learning mode should be blocked if we're already in learning mode for a different command. Also make sure there are unit tests fit this behavior, to ensure proper edge case handling.
| // Poll indefinitely: the loop exits when data is received (early return), | ||
| // the cancellation token fires (OperationCanceledException), or the device | ||
| // times out in learning mode and throws a BroadlinkException. | ||
| while (true) | ||
| { | ||
| ct.ThrowIfCancellationRequested(); | ||
|
|
||
| Logger.BroadlinkCommandService_PollingForLearnedData(command); | ||
| byte[]? data = await connection.CheckLearnedDataAsync(ct); | ||
|
|
||
| if (data is not null) | ||
| { | ||
| Logger.BroadlinkCommandService_LearnedDataReceived(command, data.Length); | ||
| string base64Data = Convert.ToBase64String(data); | ||
| _persistSettings.Set($"IRData:{command.Name}", base64Data); | ||
| command.ExecuteAsync = CreateWrappedHandler(command, sendCt => connection.SendDataAsync(data, sendCt)); | ||
| command.IsEnabled = true; | ||
| return; | ||
| } | ||
|
|
||
| await Task.Delay(pollInterval, ct); | ||
| } |
There was a problem hiding this comment.
We can add an overall timeout, but it should be configured to be much longer than the timeout of the device. Otherwise we risk leaving the device in learning mode while the application has moved on.
There was a problem hiding this comment.
Make sure there are unit tests to validate proper edge case behavior around timeouts.
| ## Overview | ||
| - **Purpose:** Persistently store user-programmed IR command payloads and other configurable settings. | ||
| - **Location:** Default path is `%LocalAppData%\AdaptiveRemote\Settings.ini` (see `ProgrammaticSettings.ProgrammaticSettingsPath`). | ||
| - **Format:** INI-style key-value pairs. | ||
| - **Key Pattern:** Command name (e.g., `Power`, `VolumeUp`, `VolumeDown`) under `[IRCommands]` section | ||
| - **Value:** Base64-encoded IR data (as returned by the Broadlink device) for IR command payloads | ||
|
|
||
| ## Example | ||
| ``` | ||
| [IRCommands] | ||
| Power = AABgA6gDAwQFBgcICQoLDA0ODw== | ||
| VolumeUp = AABgA6gDAwQFBgcICQoLDA0ODw== | ||
| VolumeDown = AABgA6gDAwQFBgcICQoLDA0ODw== | ||
| ``` |
| using AdaptiveRemote.Services.Testing; | ||
| using Microsoft.Playwright; | ||
| using static Google.Protobuf.Reflection.SourceCodeInfo.Types; | ||
|
|
| // Warm up Playwright | ||
| CurrentPage = _browserProvider.CurrentPage as IPage | ||
| ?? throw new InvalidOperationException("IBrowserProvider service did not provide an object of type IPage"); | ||
| } |
| @inject Models.ModalMessageView ModalViewModel | ||
|
|
||
| @if (CurrentMessage is not null) | ||
| { | ||
| <div class="conversation-speaking-message"><span>"@CurrentMessage"</span></div> | ||
| } | ||
|
|
||
| @code { | ||
| private string? CurrentMessage => ModalViewModel?.CurrentMessage; | ||
|
|
||
| protected override void OnInitialized() | ||
| { | ||
| if (ModalViewModel is not null) | ||
| { | ||
| ModalViewModel.PropertyChanged += (sender, args) => InvokeAsync(StateHasChanged); | ||
| } | ||
|
|
||
| base.OnInitialized(); | ||
| } |
There was a problem hiding this comment.
The modal message box should be rendering markdown to HTML, and the message should not be quoted. Note that the conversation message also needs to be markdown in order to maintain its large-text appearance, since they share the UI now.
There was a problem hiding this comment.
Make sure there are E2E tests that confirm proper behavior
| # Programmable IR Commands � Design Document | ||
|
|
| public Task ShowMessageAsync(string message, Func<CancellationToken, Task> body, bool keepAlive = false, CancellationToken cancellationToken = default) | ||
| { | ||
| TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously); | ||
| _channel.Writer.TryWrite(new(message, body, keepAlive, tcs, cancellationToken)); | ||
| return tcs.Task; |
There was a problem hiding this comment.
Use WriteAsync and propagate. Ensure there are unit tests that cover these cases to guarantee proper edge case behavior.
…eout Fix 1 (CancellationTokenSource leak in EnterLearningMode): Use Interlocked.CompareExchange to atomically guard against re-entry. If EnterLearningMode is called while already in learning mode, the new CTS is discarded immediately and the existing token is returned. This prevents the prior CancellationTokenSource from being overwritten and leaked without cancellation. Fix 2 (missing timeout on learning poll loop): Add MaxLearnDuration setting to BroadlinkSettings (default: 30 seconds). The poll loop now creates a linked CancellationTokenSource combining the caller token with a timeout token. If the device never returns learned data within MaxLearnDuration seconds, the OperationCanceledException is caught, the BroadlinkCommandService_LearningTimedOut warning is logged, and the exception is re-thrown to cancel the programming operation cleanly. New unit tests: - LifecycleViewControllerTests: verifies re-entry prevention and token lifecycle for EnterLearningMode/ExitLearningModeAsync - BroadlinkCommandServiceTests.ProgramAsync_MaxLearnDurationExpires: verifies the timeout log and task cancellation when MaxLearnDuration elapses without received data https://claude.ai/code/session_019WS6fh5i8wVUcderhHKN1Q
|
@copilot please address the comments in the most recent code review. I have added comments with fix details where needed. My comments do not mean the issues are resolved. After you have made all the changes, make sure the entire solution builds without warnings or errors, and that all tests pass. |
Working branch for ADR-127: Programmable IR Commands