Allow user to test available device macros

[vicocz]

• added macro support for Pfx Brick - simple discovery + test macro execution
• UI macro test

[Copilot]

🟡 Changes recommended

There are confirmed functional issues in the macro feature (mismatched translation keys, missing macro name resource, and command enablement not updating on connect/disconnect) plus a likely build/config risk from using the field backing keyword.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR introduces an initial “device macros” concept and UI so users can browse and execute macros exposed by connected devices (starting with BuWizz/BuWizz2 and PFx Brick), alongside refactoring PFx protocol code into the FxBricks area.

Changes:

• Add macro domain types (MacroDescriptor, MacroChoice, MacroInvocation, etc.) and surface macros from supported devices.
• Add a macros view to DevicePage with toolbar navigation between Channels / Sensors / Macros and a macro execution flow.
• Move/expand PFx protocol implementation under DeviceManagement/FxBricks and adjust tests/namespaces accordingly.

File summaries

File	Description
BrickController2/BrickController2/UI/ViewModels/MacroItemViewModel.cs	New view-model wrapper for displaying a macro descriptor with translated name.
BrickController2/BrickController2/UI/ViewModels/DevicePageViewModel.cs	Adds macro list + view switching flags/commands + macro execution dialog flow.
BrickController2/BrickController2/UI/Pages/DevicePage.xaml	Adds Macros UI section and secondary toolbar items to switch views and execute macros.
BrickController2/BrickController2/Resources/TranslationResources*.resx	Adds new macro-related translation keys and BuWizz level labels (partial locale coverage).
BrickController2/BrickController2/Protocols/PfxProtocol.cs	Removes the old PFx protocol location (moved under FxBricks).
BrickController2/BrickController2/DeviceManagement/Macros/*.cs	New macro domain model types.
BrickController2/BrickController2/DeviceManagement/FxBricks/PfxProtocol.cs	New/expanded PFx protocol implementation (file directory + sound control, parsing helpers).
BrickController2/BrickController2/DeviceManagement/FxBricks/PfxFileDirEntry.cs	New record for PFx file directory entry parsing results.
BrickController2/BrickController2/DeviceManagement/FxBricks/PfxBrickDeviceManager.cs	Namespace moved under DeviceManagement.FxBricks.
BrickController2/BrickController2/DeviceManagement/FxBricks/PfxBrickDevice.cs	Adds PFx macros (sound-file selection) and directory scan to populate choices.
BrickController2/BrickController2/DeviceManagement/DI/DeviceManagementModule.cs	Adds FxBricks using to match new namespace.
BrickController2/BrickController2/DeviceManagement/Device.cs	Adds macro capability surface to base Device.
BrickController2/BrickController2/DeviceManagement/BuwizzDevice.cs	Adds static macro for output level selection + execution hook.
BrickController2/BrickController2/DeviceManagement/BuWizz2Device.cs	Adds static macro for output level selection + execution hook.
BrickController2/BrickController2.Tests/DeviceManagement/FxBricks/PfxProtocolTests.cs	Updates references/namespaces for moved PFx protocol.
BrickController2/BrickController2.Tests/DeviceManagement/FxBricks/PfxBrickDeviceManagerTests.cs	Updates references/namespaces for moved PFx manager.

Review details

Files not reviewed (1)

• BrickController2/BrickController2/Resources/TranslationResources.Designer.cs: Generated file

Suppressed comments (5)

BrickController2/BrickController2/UI/ViewModels/DevicePageViewModel.cs:135

• These properties use the field keyword as an implicit backing field. The rest of the codebase uses explicit backing fields (e.g., UI/ViewModels/ScannerPageViewModelBase.cs), and field requires a newer language feature that may not be enabled for this project.

        public bool ShowChannelView
        {
            get;
            set
            {

BrickController2/BrickController2/UI/ViewModels/DevicePageViewModel.cs:147

• These properties use the field keyword as an implicit backing field. The rest of the codebase uses explicit backing fields (e.g., UI/ViewModels/ScannerPageViewModelBase.cs), and field requires a newer language feature that may not be enabled for this project.

        public bool ShowMacroView
        {
            get;
            set
            {

BrickController2/BrickController2/DeviceManagement/FxBricks/PfxBrickDevice.cs:17

• PlaySoundMacroNameKey references a translation key ("PfxPlaySoundMacroName") that isn't present in the translation resources, so the UI will display the raw key text.
  BrickController2/BrickController2/DeviceManagement/FxBricks/PfxBrickDevice.cs:275
• A PlaySound macro is added even when no sound files are found (choices is empty). In that case the macro appears in the UI but does nothing when executed (choiceValue stays null and ExecuteMacroAsync completes without writing).
  BrickController2/BrickController2/DeviceManagement/FxBricks/PfxBrickDevice.cs:75
• invocation.ChoiceValue is the string "soundId" (FileId.ToString()), not a file name; naming it fileName is misleading and makes the lookup logic harder to follow.

• Files reviewed: 22/23 changed files
• Comments generated: 5
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Macro labels, PFx file filtering, empty macro behavior, and command-state updates contain user-visible defects.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files not reviewed (1)

• BrickController2/BrickController2/Resources/TranslationResources.Designer.cs: Generated file

Suppressed comments (4)

BrickController2/BrickController2/DeviceManagement/BuwizzDevice.cs:36

• These label keys do not match the resources added by this PR (BuWizz_Low, BuWizz_Normal, and BuWizz_High). TranslationHelper therefore falls back to displaying the literal MacroChoice_BuWizz_* identifiers in the selection dialog. Use the resource keys that actually exist.

                    new MacroChoice<int>("MacroChoice_BuWizz_Low", (int)BuWizzOutputLevels.Low),
                    new MacroChoice<int>("MacroChoice_BuWizz_Normal", (int)BuWizzOutputLevels.Normal),
                    new MacroChoice<int>("MacroChoice_BuWizz_High", (int)BuWizzOutputLevels.High)

BrickController2/BrickController2/DeviceManagement/BuWizz2Device.cs:40

• These label keys do not match the newly added BuWizz_* resources, so the choice dialog displays the raw MacroChoice_BuWizz_* identifiers instead of translated labels. Use the existing resource keys.

                    new MacroChoice<int>("MacroChoice_BuWizz_Low", (int)BuWizz2OutputLevels.Low),
                    new MacroChoice<int>("MacroChoice_BuWizz_Normal", (int)BuWizz2OutputLevels.Normal),
                    new MacroChoice<int>("MacroChoice_BuWizz_High", (int)BuWizz2OutputLevels.High),
                    new MacroChoice<int>("MacroChoice_BuWizz_Ludicrous", (int)BuWizz2OutputLevels.Ludicrous),

BrickController2/BrickController2/DeviceManagement/FxBricks/PfxBrickDevice.cs:314

• Every valid directory entry is currently offered as playable audio, including text, archive, image, and configuration files represented by PfxFileFormat. Also, converting the 16-bit file ID to byte silently redirects IDs above 255 to another file. Restrict choices to supported audio formats and IDs representable by the sound command.
  BrickController2/BrickController2/DeviceManagement/FxBricks/PfxBrickDevice.cs:317
• Play/stop descriptors are added even when the device has no selectable sound files. Since the UI skips selection for an empty choice list, pressing either macro then reports success while ExecuteMacroAsync silently performs no operation. Only expose these macros when at least one sound choice exists.

• Files reviewed: 22/23 changed files
• Comments generated: 4
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

PFx audio choices cannot currently resolve to file IDs, and protocol/write failures can be reported as successful.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files not reviewed (1)

• BrickController2/BrickController2/Resources/TranslationResources.Designer.cs: Generated file

Suppressed comments (8)

Previously missed (3) — in code that hasn't changed since the last review.

BrickController2/BrickController2/DeviceManagement/FxBricks/PfxBrickDevice.cs:95

• PFx macro branches return WriteCommandAsync as a non-generic Task, discarding its Boolean result. Since that helper catches every exception and returns false, BLE write failures look successful and the new UI's failure dialog is never reached. Await each write and throw/report when it returns false.
  BrickController2/BrickController2/DeviceManagement/FxBricks/PfxProtocol.cs:165
• This parser accepts responses whose RequestStatus byte indicates failure, provided the opcode and length match, and can therefore turn an error payload into a directory entry. Validate data[1] against the protocol's success status before reading fields.

This issue also appears on line 194 of the same file.
BrickController2/BrickController2/DeviceManagement/FxBricks/PfxProtocol.cs:208

• Both cref targets omit the EVT_ prefix and do not exist, so these API docs produce unresolved references instead of links to the declared constants.

BrickController2/BrickController2/DeviceManagement/FxBricks/PfxBrickDevice.cs:313

• The selected choice carries the file name, but _macroFileIds is keyed by the decimal file ID below. Consequently every Play/Stop lookup fails and both macros silently do nothing. Keep the file name as the label and use the file ID string as the choice value.
  BrickController2/BrickController2/DeviceManagement/FxBricks/PfxBrickDevice.cs:320
• When the device has no audio files, these Play/Stop descriptors are still exposed with zero choices. MacroItemViewModel then skips selection, passes null, and ExecuteMacroAsync completes without sending anything, leaving enabled buttons that are guaranteed no-ops. Add these descriptors only when at least one audio choice exists.
  BrickController2/BrickController2/DeviceManagement/FxBricks/PfxProtocol.cs:197
• The documented RequestStatus at byte 1 is ignored here, so a well-sized error response is interpreted as a valid file count and can trigger an incorrect directory scan. Reject non-success statuses before reading the trailing count.

        if (data.Length < 4 || data[0] != RSP_FILE_DIR)
        {
            return null;
        }

BrickController2/BrickController2/UI/ViewModels/MacroItemViewModel.cs:32

• Math.Abs(int.MinValue) throws OverflowException, and a string hash may legally equal int.MinValue; merely rendering that macro would then fail. Mask off the sign bit instead of calling Abs.

    public int Idx => Math.Abs(_descriptor.Id.GetHashCode());

BrickController2/BrickController2/UI/ViewModels/MacroItemViewModel.cs:78

• The translated resource already ends with a colon, so adding another ": " renders messages such as Execution of the macro has failed:: .... Preserve the resource punctuation and append only a space.

                Translate("ExecuteMacroFailed") + ": " + ex.Message,

• Files reviewed: 21/22 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

PFx audio choices cannot resolve their file IDs, causing play and stop macros to silently do nothing.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files not reviewed (1)

• BrickController2/BrickController2/Resources/TranslationResources.Designer.cs: Generated file

Suppressed comments (5)

Previously missed (1) — in code that hasn't changed since the last review.

BrickController2/BrickController2/Resources/TranslationResources.resx:679

• This text already ends with a colon, while MacroItemViewModel appends another ": " before the exception message, producing failed:: .... Remove the colon from the resource value.

BrickController2/BrickController2/DeviceManagement/FxBricks/PfxBrickDevice.cs:317

• The choice stores the file name, but _macroFileIds is keyed by the numeric ID string. Consequently, both play and stop lookups always fail and ExecuteMacroAsync silently returns without sending a command. Store the ID string as the choice value while retaining the file name as its label.
  BrickController2/BrickController2/DeviceManagement/FxBricks/PfxBrickDevice.cs:321
• When the device has no audio files, these play/stop descriptors still expose executable buttons with zero choices. MacroItemViewModel then invokes them with null, and the device silently performs no operation. Only add both file-based macros when at least one audio choice exists (or explicitly disable zero-choice macros).
  BrickController2/BrickController2/Protocols/PfxProtocol.cs:157
• The new binary parser has no coverage, although PfxProtocolTests verifies exact protocol payloads for the existing helpers. Add cases for a valid big-endian directory response, truncated/wrong-opcode input, name trimming, and file-format classification; these byte offsets are otherwise easy to regress unnoticed.

    public static FileDirEntry? ParseFileDirEntry(byte[] data)

BrickController2/BrickController2/UI/ViewModels/MacroItemViewModel.cs:27

• This command's availability depends on DeviceState, but no CanExecuteChanged notification is raised when the device disconnects. The button therefore remains enabled with its last connected-state result and can invoke a macro while disconnected. Expose a refresh method on this item and call it for every macro from the page's existing connection/disconnection command update path.

        ExecuteMacroCommand = new SafeCommand(ExecuteMacroAsync, () => _device.DeviceState == DeviceState.Connected && !_dialogService.IsDialogOpen);

• Files reviewed: 19/20 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

Zero-choice PFx macros silently succeed, and macro command state and localization remain incomplete.

Review details

Files not reviewed (1)

• BrickController2/BrickController2/Resources/TranslationResources.Designer.cs: Generated file

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

BrickController2/BrickController2/Protocols/PfxProtocol.cs:209

• These XML references point to nonexistent constants; the declared names are EVT_SOUNDFX_RETRIGGER_TOGGLE and EVT_SOUNDFX_RETRIGGER_RESTART. This leaves broken API documentation links and can emit CS1574 when XML documentation warnings are enabled.

BrickController2/BrickController2/UI/ViewModels/DevicePageViewModel.cs:516

• The macro commands are omitted from the connection-state refresh. After a disconnect, each button can therefore remain visually enabled even though its predicate now returns false. Raise CanExecuteChanged for every macro command here as well.

            SwitchToChannelViewCommand.RaiseCanExecuteChanged();
            SwitchToSensorViewCommand.RaiseCanExecuteChanged();
            SwitchToMacroViewCommand.RaiseCanExecuteChanged();

BrickController2/BrickController2/DeviceManagement/FxBricks/PfxBrickDevice.cs:323

• When the PFx Brick contains no audio files, these descriptors have zero choices, but the UI still enables their Run buttons. Execution then passes null, matches no branch in ExecuteMacroAsync, and reports apparent success without sending a command. Only expose these file-dependent macros when at least one audio choice exists (or explicitly disable zero-choice macros).
  BrickController2/BrickController2/UI/ViewModels/MacroItemViewModel.cs:36
• Neither OneShot nor Repeatable exists in any translation resource, so this translation call always falls back to the raw enum identifier (including for German and Hungarian users). Add resource entries for both enum values or map them to defined localization keys.

    public string Kind => Translate(_descriptor.Kind);

• Files reviewed: 20/21 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Directory validation and execution-result handling can expose invalid macros or silently report failed commands as successful.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files not reviewed (1)

• BrickController2/BrickController2/Resources/TranslationResources.Designer.cs: Generated file

Suppressed comments (1)

BrickController2/BrickController2/DeviceManagement/FxBricks/PfxBrickDevice.cs:321

• Play and stop descriptors are added even when discovery found no audio files. They then have zero choices, so the UI exposes executable-looking macros that pass null and always return false. Only publish these descriptors when audioFilesChoices is nonempty (or model them as unavailable/disabled).

• Files reviewed: 21/22 changed files
• Comments generated: 2
• Review effort level: Balanced