diff --git a/AGENTS.md b/AGENTS.md index 6c6724f..176d6fb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -76,17 +76,21 @@ src/DeviceBatteryInfo/ scanning. Hid/ has the shared HID transport/interop, HidProtocol (what a brand implements) and HidFamily; Razer/RazerProtocol.cs lists the supported mice - Variables/BatteryVariableCatalog.cs slot x field -> VariableDefinition, and the reverse resolve + Variables/BatteryVariableCatalog.cs slot -> its folder (`devices/`), slot x field -> + VariableDefinition (in that folder), the search match, and the reverse + resolve Localization/Strings.resx default-culture strings; Strings..resx per language Assets/icon.svg Properties/launchSettings.json the single real-host debug profile tests/DeviceBatteryInfo.Tests/ - BatteryIntegrationTests.cs builds, initializes, the variables catalogue + a read work + BatteryIntegrationTests.cs builds, initializes, the variables catalogue (folders, search, folder + ids, pinned variable ids and names) + a read work BatterySourceParsingTests.cs Razer report / PnP / Win32 power-status parsers, the Android picker BatteryTrendTrackerTests.cs trend windows, segments per charging state, the trend text BatteryWidgetViewTests.cs every widget tree built through a real UiView, the config form, glyph paths, responsive variants, the previews - CatalogNotificationTests.cs the integration never takes a catalog notifier, and re-initializes cleanly + CatalogNotificationTests.cs the integration never takes a catalog notifier, re-initializes cleanly, + and reloads the variable catalog only when the devices change DeviceConfigFlowTests.cs the config flow end to end, the way the host drives it DeviceEntryReaderTests.cs config entries -> device slots, including the legacy keys DeviceFamilyTests.cs model ids, the catalog, DeviceFamilyProvider @@ -127,6 +131,25 @@ Design knowledge that is not obvious from the code alone: empty and let the host reach each `battery__` id through `DiscoverAsync`/`ResolveAsync` instead. `SupportsPush` + `OnAttachedAsync` still push live updates to an attached session; `ReadAsync` answers a direct `get`. + **The catalog is a tree: one folder per device.** The host draws folders only for a provider with + `SupportsSearch => true`; without it, it walks every container and lists the leaves flat. With it, + `DiscoverAsync` answers three shapes: no parent and no search gives the folders (`IsContainer`, not + bindable, id `devices/`), a folder id as `ParentId` gives that device's variables (each with + that `ParentId`), and no parent with a search gives the matching variables of every device, never a + folder: every word typed (split on space, `_` and `.`, the host strips a leading `vars.`) must appear + in the variable's `Name` or the device's display name, ignoring case. A slug never has a `/`, so a + folder id can never be a variable id; `TryResolve` rejects it, so a read or subscribe of a folder is + unavailable, and `ResolveAsync` returns the folder. The host stores a link as the variable's `Id` plus a + copy of its name (`dynamic-variable-bindings.json`), so the ids and names stay exactly + `-` and `battery__`; the folder is only a `ParentId`. The host keeps the tree + it browsed last, even across a plugin restart, until `IVariableSink.InvalidateCatalogAsync`: the + integration calls it in `OnAttachedAsync` and whenever the device ids or display names change + (`DeviceCatalog.Changed`, or a registry source outside the config appearing, going or renamed), + compared against the set it last announced, so readings never trigger one. It only signals the host's + browser to re-discover, which never changes the device set, so it cannot loop the way + `CatalogChanged` does. `CatalogEntryCount` counts the bindable variables, not the folders. A device + name is passed to the variable strings as a plain argument; a localized string as the argument would + render as `{placeholder}` in the host. - **`DeviceCatalog` is the single source of device identity at runtime.** Providers, the variable catalogue and the widgets all read `DeviceCatalog.Devices`. It starts empty and is populated only by `BatteryIntegration.InitializeAsync` from the config-flow entries (`DeviceEntryReader`) - there is no @@ -854,8 +877,9 @@ integration, which re-enters `InitializeAsync`, which notifies again - an 8-deep every provided variable failing to register as `AlreadyExists` (net: zero variables, and the localization + widget-type registration churned so strings render as `[[plugin::Key]]` and the widgets vanish). `BatteryIntegration` therefore takes no `IPluginCatalogNotifier` at all -(`CatalogNotificationTests` asserts that): a device-set change only re-polls, and the on-demand variable -catalogue is resolved live. Integration issues need no notification either, because the host lists them +(`CatalogNotificationTests` asserts that): a device-set change only re-polls and reloads the variable +browser through `IVariableSink.InvalidateCatalogAsync` (a different, loop-free signal, see the catalog +note under Orientation), and the on-demand variable catalogue is resolved live. Integration issues need no notification either, because the host lists them with a live call on its own schedule (confirmed: the Linux HID issue appears without one). ### Configuration and secrets diff --git a/README.md b/README.md index 377e776..dc814b9 100644 --- a/README.md +++ b/README.md @@ -41,6 +41,10 @@ Widgets update live between polls. The battery trend is shown as a signed change covers, for example `-13%/1h` while discharging or `+28%/30m` while charging. It needs a couple of minutes of history and starts over whenever the plugin restarts. +**Variables** for every device you add: `battery__percent`, `_status`, `_charging`, `_online`, +`_time_to_full`, `_trend` and `_trend_rate`. Macro Deck's variable list shows them in one folder per +device, and its search box finds them across devices by name and device name (`phone percent`, say). + **Events** you can use in flows: `charging-started`, `charging-stopped` and `battery-low`. **A "Refresh battery levels" action** that forces an immediate poll. diff --git a/src/DeviceBatteryInfo/BatteryIntegration.cs b/src/DeviceBatteryInfo/BatteryIntegration.cs index 9d6deee..0e84ec2 100644 --- a/src/DeviceBatteryInfo/BatteryIntegration.cs +++ b/src/DeviceBatteryInfo/BatteryIntegration.cs @@ -44,6 +44,7 @@ ILogger logger private IIntegrationContext? _context; private IVariableSink? _sink; + private string? _catalogDevices; public IReadOnlyList Actions { get; } = [new RefreshBatteryAction(polling)]; @@ -94,15 +95,13 @@ public Task ShutdownAsync() DetachBatteryCatalog(); _context = null; _sink = null; + _catalogDevices = null; return Task.CompletedTask; } // Empty on purpose: every variable is served through the catalog below (SupportsCatalog), not this list. public IReadOnlyList Variables => Array.Empty(); - private IReadOnlyList CatalogDefinitions() => - BatteryVariableCatalog.Build(CurrentSlots()); - // Configured devices plus any the registry has seen, so a contributed provider needs no config flow. private List CurrentSlots() { @@ -131,12 +130,47 @@ private void OnDeviceCatalogChanged(object? sender, EventArgs e) // Just re-poll. Never call IPluginCatalogNotifier.CatalogChanged from here: it re-enters // InitializeAsync, which would call it again: an infinite re-registration loop. _polling.RequestRefresh(); + InvalidateCatalogIfDevicesChanged(); + } + + // The host keeps the catalog it browsed last, even across a plugin restart, until told to reload it. + private void InvalidateCatalogIfDevicesChanged() + { + var sink = _sink; + if (sink is null) + { + return; + } + + var devices = DeviceSignature(); + if (Interlocked.Exchange(ref _catalogDevices, devices) != devices) + { + _ = InvalidateCatalogAsync(sink, CancellationToken.None); + } + } + + private string DeviceSignature() => + string.Join('\n', CurrentSlots().Select(slot => $"{slot.Id}\t{slot.DisplayName}")); + + private async Task InvalidateCatalogAsync(IVariableSink sink, CancellationToken cancellationToken) + { + try + { + await sink.InvalidateCatalogAsync(cancellationToken); + } + catch (Exception exception) when (exception is not OperationCanceledException) + { + _logger.Debug(exception, "Invalidating the variable catalog failed."); + } } public bool SupportsCatalog => true; public bool SupportsPush => true; + // Without search the host flattens the catalog and lists no folders. + public bool SupportsSearch => true; + public string CatalogName => "Battery"; public int? CatalogEntryCount => CurrentSlots().Count * BatteryVariableCatalog.FieldsPerDevice; @@ -144,7 +178,8 @@ private void OnDeviceCatalogChanged(object? sender, EventArgs e) public Task OnAttachedAsync(IVariableSink sink, CancellationToken cancellationToken = default) { _sink = sink; - return Task.CompletedTask; + Volatile.Write(ref _catalogDevices, DeviceSignature()); + return InvalidateCatalogAsync(sink, cancellationToken); } public ValueTask DiscoverAsync( @@ -152,19 +187,25 @@ public ValueTask DiscoverAsync( CancellationToken cancellationToken = default ) { - IEnumerable defs = CatalogDefinitions(); - if (!string.IsNullOrWhiteSpace(query.Search)) + var slots = CurrentSlots(); + VariableDefinition[] ordered = query switch { - defs = defs.Where(d => - ( - d.Name is { } name - && name.Contains(query.Search, StringComparison.OrdinalIgnoreCase) - ) - || (d.Id is { } id && id.Contains(query.Search, StringComparison.OrdinalIgnoreCase)) - ); - } + { ParentId: null, Search: { } search } when !string.IsNullOrWhiteSpace(search) => + [ + .. slots.SelectMany(slot => + BatteryVariableCatalog + .ForDevice(slot) + .Where(variable => BatteryVariableCatalog.Matches(variable, slot, search)) + ), + ], + { ParentId: null } => [.. slots.Select(BatteryVariableCatalog.Folder)], + _ => slots.Find(slot => + BatteryVariableCatalog.FolderId(slot.Id) == query.ParentId + ) is { } parent + ? [.. BatteryVariableCatalog.ForDevice(parent)] + : [], + }; - var ordered = defs.ToArray(); var start = int.TryParse(query.ContinuationToken, out var parsed) ? Math.Clamp(parsed, 0, ordered.Length) : 0; @@ -189,7 +230,12 @@ d.Name is { } name CancellationToken cancellationToken = default ) => ValueTask.FromResult( - CatalogDefinitions() + CurrentSlots() + .SelectMany(slot => + BatteryVariableCatalog + .ForDevice(slot) + .Prepend(BatteryVariableCatalog.Folder(slot)) + ) .FirstOrDefault(d => string.Equals(d.ResolvedId, localId, StringComparison.Ordinal)) ); @@ -300,6 +346,11 @@ private VariableReading ReadField(BatteryVariableCatalog.Field field, BatterySna private void OnSnapshotChanged(object? sender, BatterySnapshotChangedEventArgs e) { + if (e.Previous?.DisplayName != e.Current?.DisplayName) + { + InvalidateCatalogIfDevicesChanged(); + } + if (e.Current is not { } current) { return; diff --git a/src/DeviceBatteryInfo/Variables/BatteryVariableCatalog.cs b/src/DeviceBatteryInfo/Variables/BatteryVariableCatalog.cs index d7c2583..a3e11f9 100644 --- a/src/DeviceBatteryInfo/Variables/BatteryVariableCatalog.cs +++ b/src/DeviceBatteryInfo/Variables/BatteryVariableCatalog.cs @@ -36,18 +36,34 @@ private static readonly (Field Field, string Suffix)[] SuffixesByLength = .. Fields.Select(f => (f.Field, f.Suffix)).OrderByDescending(f => f.Suffix.Length), ]; - public static IReadOnlyList Build(IReadOnlyList slots) - { - var definitions = new List(slots.Count * Fields.Length); - foreach (var slot in slots) + // A slug never contains '/', so a folder id can never be a variable id. + private const string FolderIdPrefix = "devices/"; + + private static readonly char[] SearchSeparators = [' ', '_', '.']; + + public static IReadOnlyList Build(IReadOnlyList slots) => + [.. slots.SelectMany(ForDevice)]; + + public static IReadOnlyList ForDevice(BatterySlot slot) => + [.. Fields.Select(f => Define(slot, f.Field, f.Suffix, f.Type))]; + + public static string FolderId(string slotId) => FolderIdPrefix + slotId; + + public static VariableDefinition Folder(BatterySlot slot) => + VariableDefinition.OnDemand(FolderId(slot.Id), VariableType.Text) with { - foreach (var (field, suffix, type) in Fields) - { - definitions.Add(Define(slot, field, suffix, type)); - } - } + DisplayName = slot.DisplayName, + IsContainer = true, + IsBindable = false, + }; - return definitions; + // Every word typed must appear in the variable's name or its device's name ("phone percent"). + public static bool Matches(VariableDefinition variable, BatterySlot slot, string search) + { + var text = $"{variable.Name} {slot.DisplayName}"; + return search + .Split(SearchSeparators, StringSplitOptions.RemoveEmptyEntries) + .All(word => text.Contains(word, StringComparison.OrdinalIgnoreCase)); } public static IEnumerable<(string LocalId, Field Field)> FieldsFor(string slotId) => @@ -55,6 +71,13 @@ public static IReadOnlyList Build(IReadOnlyList public static bool TryResolve(string localId, out string slotId, out Field field) { + slotId = string.Empty; + field = default; + if (localId.StartsWith(FolderIdPrefix, StringComparison.Ordinal)) + { + return false; + } + foreach (var (candidate, suffix) in SuffixesByLength) { if ( @@ -68,8 +91,6 @@ public static bool TryResolve(string localId, out string slotId, out Field field } } - slotId = string.Empty; - field = default; return false; } @@ -120,6 +141,7 @@ VariableType type Name = $"battery_{slot.Id.Replace('-', '_')}_{suffix.Replace('-', '_')}", Type = type, Materialization = VariableMaterialization.OnDemand, + ParentId = FolderId(slot.Id), DisplayName = displayName, Description = description, DecimalPlaces = field switch diff --git a/tests/DeviceBatteryInfo.Tests/BatteryIntegrationTests.cs b/tests/DeviceBatteryInfo.Tests/BatteryIntegrationTests.cs index b6d107d..b7a08ce 100644 --- a/tests/DeviceBatteryInfo.Tests/BatteryIntegrationTests.cs +++ b/tests/DeviceBatteryInfo.Tests/BatteryIntegrationTests.cs @@ -1,6 +1,7 @@ using DeviceBatteryInfo.Core; using DeviceBatteryInfo.HostBatteries; using DeviceBatteryInfo.Sources; +using DeviceBatteryInfo.Variables; using MacroDeck.Plugin.Testing; using MacroDeck.Sdk; using MacroDeck.Sdk.Variables; @@ -118,11 +119,11 @@ public async Task The_variable_catalog_is_discoverable_but_not_eagerly_declared( var describeJson = describe.Data?.ToString() ?? string.Empty; Assert.That(describeJson, Does.Not.Contain("battery_system_percent")); - var discover = await harness.Variables.DiscoverAsync(pageSize: 200); + var discover = await harness.Variables.DiscoverAsync(search: "percent", pageSize: 200); Assert.That(discover.Succeeded, Is.True, discover.Error?.ToString() ?? "none"); var discoverJson = discover.Data?.ToString() ?? string.Empty; Assert.That(discoverJson, Does.Contain("system-percent")); - Assert.That(discoverJson, Does.Contain("headset-charging")); + Assert.That(discoverJson, Does.Contain("headset-percent")); var resolve = await harness.Variables.ResolveAsync("system-percent"); Assert.That(resolve.Succeeded, Is.True, resolve.Error?.ToString() ?? "none"); @@ -172,7 +173,7 @@ public async Task The_variable_set_is_on_demand_so_the_host_never_bulk_registers } var page = await integration.DiscoverAsync( - new VariableCatalogQuery { PageSize = 3 } + new VariableCatalogQuery { ParentId = "devices/system", PageSize = 3 } ); using (Assert.EnterMultipleScope()) { @@ -187,6 +188,7 @@ public async Task The_variable_set_is_on_demand_so_the_host_never_bulk_registers var next = await integration.DiscoverAsync( new VariableCatalogQuery { + ParentId = "devices/system", PageSize = 3, ContinuationToken = page.ContinuationToken, } @@ -197,6 +199,202 @@ public async Task The_variable_set_is_on_demand_so_the_host_never_bulk_registers ); } + private static BatteryIntegration SeedPhoneAndLaptop(PluginTestHarness harness) + { + harness + .Services.GetRequiredService() + .Set( + [ + new BatterySlot( + "phone", + "Pixel 8", + BatterySourceKind.Phone, + DeviceType.AdbPhone, + AdbAddress: "1.2.3.4:5555" + ), + new BatterySlot( + "laptop", + "Work Laptop", + BatterySourceKind.System, + DeviceType.System + ), + ] + ); + + return harness.Services.GetRequiredService(); + } + + [Test] + public async Task The_top_level_is_one_folder_per_device() + { + await using var harness = CreateHarness(); + await harness.InitializeIntegrationsAsync(); + var integration = SeedPhoneAndLaptop(harness); + + var page = await integration.DiscoverAsync(new VariableCatalogQuery()); + + using (Assert.EnterMultipleScope()) + { + Assert.That( + page.Items.Select(i => i.Id), + Is.EqualTo(new[] { "devices/phone", "devices/laptop" }) + ); + Assert.That( + page.Items.Select(i => i.DisplayName.Literal), + Is.EqualTo(new[] { "Pixel 8", "Work Laptop" }) + ); + Assert.That(page.Items.Select(i => i.IsContainer), Is.All.True); + Assert.That(page.Items.Select(i => i.IsBindable), Is.All.False); + Assert.That(page.ContinuationToken, Is.Null); + } + } + + [Test] + public async Task A_folder_holds_exactly_its_devices_variables() + { + await using var harness = CreateHarness(); + await harness.InitializeIntegrationsAsync(); + var integration = SeedPhoneAndLaptop(harness); + + var page = await integration.DiscoverAsync( + new VariableCatalogQuery { ParentId = "devices/phone" } + ); + var unknown = await integration.DiscoverAsync( + new VariableCatalogQuery { ParentId = "devices/tablet" } + ); + + using (Assert.EnterMultipleScope()) + { + Assert.That(page.Items, Has.Count.EqualTo(BatteryVariableCatalog.FieldsPerDevice)); + Assert.That(page.Items.Select(i => i.Id), Is.All.StartsWith("phone-")); + Assert.That(page.Items.Select(i => i.ParentId), Is.All.EqualTo("devices/phone")); + Assert.That(page.Items.Select(i => i.IsContainer), Is.All.False); + Assert.That(unknown.Items, Is.Empty); + } + } + + [TestCase("phone percent")] + [TestCase("PIXEL percent")] + [TestCase("battery_phone_percent")] + [TestCase("battery.phone.percent")] + public async Task A_search_lists_the_matching_variables_across_devices(string search) + { + await using var harness = CreateHarness(); + await harness.InitializeIntegrationsAsync(); + var integration = SeedPhoneAndLaptop(harness); + + var page = await integration.DiscoverAsync(new VariableCatalogQuery { Search = search }); + + Assert.That(page.Items.Select(i => i.Id), Is.EqualTo(new[] { "phone-percent" })); + } + + [Test] + public async Task A_search_never_lists_folders_or_another_devices_variables() + { + await using var harness = CreateHarness(); + await harness.InitializeIntegrationsAsync(); + var integration = SeedPhoneAndLaptop(harness); + + var percents = await integration.DiscoverAsync( + new VariableCatalogQuery { Search = "percent" } + ); + var laptop = await integration.DiscoverAsync( + new VariableCatalogQuery { Search = "work charging" } + ); + + using (Assert.EnterMultipleScope()) + { + Assert.That( + percents.Items.Select(i => i.Id), + Is.EqualTo(new[] { "phone-percent", "laptop-percent" }) + ); + Assert.That(percents.Items.Select(i => i.IsContainer), Is.All.False); + Assert.That(laptop.Items.Select(i => i.Id), Is.EqualTo(new[] { "laptop-charging" })); + } + } + + [Test] + public async Task A_folder_id_resolves_to_its_folder_and_never_to_a_variable() + { + await using var harness = CreateHarness(); + await harness.InitializeIntegrationsAsync(); + // A slot id ending in a field suffix, so the folder id would parse as that field without the guard. + harness + .Services.GetRequiredService() + .Set( + [new BatterySlot("pen-online", "Pen", BatterySourceKind.Pen, DeviceType.System)] + ); + var integration = harness.Services.GetRequiredService(); + + var folder = await integration.ResolveAsync("devices/pen-online"); + var variable = await integration.ResolveAsync("pen-online-online"); + var reading = await integration.ReadAsync("devices/pen-online"); + var subscribed = await integration.SubscribeAsync(["devices/pen-online"]); + + using (Assert.EnterMultipleScope()) + { + Assert.That(folder?.IsContainer, Is.True); + Assert.That(folder?.IsBindable, Is.False); + Assert.That(variable?.IsContainer, Is.False); + Assert.That( + BatteryVariableCatalog.TryResolve("devices/pen-online", out _, out _), + Is.False + ); + Assert.That(reading, Is.EqualTo(VariableReading.Unavailable)); + Assert.That( + subscribed.Single(), + Is.EqualTo(VariableValue.Unavailable("devices/pen-online")) + ); + } + } + + [Test] + public void Variable_ids_and_names_of_a_slot_stay_as_released() + { + var slot = new BatterySlot( + "gaming-headset", + "Gaming Headset", + BatterySourceKind.Headset, + DeviceType.Bluetooth + ); + + var variables = BatteryVariableCatalog.ForDevice(slot); + + using (Assert.EnterMultipleScope()) + { + Assert.That( + variables.Select(v => v.Id), + Is.EqualTo( + new[] + { + "gaming-headset-percent", + "gaming-headset-status", + "gaming-headset-charging", + "gaming-headset-online", + "gaming-headset-time-to-full", + "gaming-headset-trend", + "gaming-headset-trend-rate", + } + ) + ); + Assert.That( + variables.Select(v => v.Name), + Is.EqualTo( + new[] + { + "battery_gaming_headset_percent", + "battery_gaming_headset_status", + "battery_gaming_headset_charging", + "battery_gaming_headset_online", + "battery_gaming_headset_time_to_full", + "battery_gaming_headset_trend", + "battery_gaming_headset_trend_rate", + } + ) + ); + } + } + private sealed class SeedSource(string id, string name) : IBatterySource { public string Id => id; diff --git a/tests/DeviceBatteryInfo.Tests/CatalogNotificationTests.cs b/tests/DeviceBatteryInfo.Tests/CatalogNotificationTests.cs index 25cd6d5..a0d97b6 100644 --- a/tests/DeviceBatteryInfo.Tests/CatalogNotificationTests.cs +++ b/tests/DeviceBatteryInfo.Tests/CatalogNotificationTests.cs @@ -55,6 +55,58 @@ public async Task A_runtime_source_appearing_after_init_never_throws() await Task.Delay(200); } + [Test] + public async Task The_variable_catalog_reloads_only_when_the_devices_change() + { + var integration = Build(out var catalog, out var registry); + var sink = new FakeVariableSink(); + await integration.InitializeAsync(new FakeIntegrationContext()); + + await integration.OnAttachedAsync(sink); + Assert.That(sink.Invalidations, Is.EqualTo(1), "attaching reloads the host's cached catalog"); + + catalog.Set( + [new BatterySlot("laptop", "Laptop", BatterySourceKind.System, DeviceType.System)] + ); + Assert.That(sink.Invalidations, Is.EqualTo(2), "a device was added"); + + catalog.Set( + [new BatterySlot("laptop", "Laptop", BatterySourceKind.System, DeviceType.System)] + ); + registry.Update( + new StubSource("laptop", "Laptop"), + BatteryReading.FromPercent(50, BatteryStatus.Discharging) + ); + registry.Update( + new StubSource("laptop", "Laptop"), + BatteryReading.FromPercent(49, BatteryStatus.Discharging) + ); + await integration.InitializeAsync(new FakeIntegrationContext()); + Assert.That( + sink.Invalidations, + Is.EqualTo(2), + "readings and re-initializing change no device" + ); + + catalog.Set( + [ + new BatterySlot( + "laptop", + "Work Laptop", + BatterySourceKind.System, + DeviceType.System + ), + ] + ); + Assert.That(sink.Invalidations, Is.EqualTo(3), "a device was renamed"); + + registry.Update( + new StubSource("addon", "Add-on"), + BatteryReading.FromPercent(50, BatteryStatus.Discharging) + ); + Assert.That(sink.Invalidations, Is.EqualTo(4), "an unconfigured source appeared"); + } + private static BatteryIntegration Build(out DeviceCatalog catalog, out BatteryRegistry registry) { var options = Options.Create(new BatteryPluginOptions());