Skip to content

The channel colour picker can never show a channel's real colour — the auto palette (38) and the picker palette (8) are disjoint #430

Open

Description

@tylerkron

The channel colour picker in the Channels settings drawer shows 8 swatches. The colour a channel is actually given when it is created comes from a different, disjoint 38-colour palette. So on a freshly created channel:

  • the "current colour" selection ring never appears on any swatch — the drawer looks like no colour is set;
  • the colour the channel is genuinely drawn in is not offered anywhere in the UI, so once the user picks any swatch the original colour is unreachable for the rest of the app's life.

This was found while scoping a different question (see The layering question I was actually sent to scope, at the bottom — its answer is "not worth doing", and that half is recorded here so nobody re-derives it).

Evidence

There are three independent channel-colour palettes

# owner type entries what it feeds
1 Daqifi.Avalonia/Daqifi.Desktop/Channel/ChannelColorManager.cs List<IBrush> of ImmutableSolidColorBrush 38 (grep -c 'ImmutableSolidColorBrush(Color.Parse' → 38), sourced from Channel/MaterialColors.cs (grep -c 'public const string' → 38) NewColor(), i.e. the colour every channel is born with
2 Daqifi.Avalonia/Daqifi.Desktop/ViewModels/ChannelsPaneViewModel.cs:99 Brush[] of ImmutableSolidColorBrush 8 inline hexes Palette, i.e. the swatches the user is offered — desktop View/Prototype/ChannelsPanePrototype.axaml:614 and mobile Views/Mobile/ChannelsMobileView.axaml:400
3 Daqifi.Avalonia/Views/MobileShellViewModel.cs:128 uint[] 8 mobile LivePlot series colours, assigned by index (Palette[i % Palette.Length], line 423) — unrelated to either of the other two

Palette 3 is out of scope below; it is listed because it is the third answer to "what colour is this channel", and it is why the fix is "one owner", not "two palettes agreeing".

Palettes 1 and 2 are disjoint

The picker's 8 hexes are #4A9EFF #4ADE80 #F59E0B #F43F5E #A855F7 #06B6D4 #EC4899 #FACC15.

grep -icE '4A9EFF|4ADE80|F59E0B|F43F5E|A855F7|06B6D4|EC4899|FACC15|D32F2F' \
  Daqifi.Avalonia/Daqifi.Desktop/Channel/MaterialColors.cs
→ 1

That 1 is the positive control D32F2F (MaterialColors.Red700) appended to the pattern on purpose. The eight picker hexes contribute zero matches. Both sides are 6-digit hexes parsed by Color.Parse, so both are opaque and equality is not an alpha artefact.

The consequence, read off the binding

View/Prototype/ChannelsPanePrototype.axaml:623-632 — the selection ring's visibility:

<Border CornerRadius="5" BorderBrush="{DynamicResource TextPrimary}" BorderThickness="2">
  <Border.IsVisible>
    <MultiBinding Converter="{StaticResource BrushColorMatch}">
      <Binding/>                                                              <!-- a Palette brush -->
      <Binding Path="$parent[UserControl].DataContext.SelectedChannel.ChannelColorBrush"/>
    </MultiBinding>
  </Border.IsVisible>
</Border>

Converters/BrushColorMatchConverter.cs returns a.Color == b.Color. The channel's brush came from palette 1; every swatch comes from palette 2; the two sets share no colour — so every ring is hidden until the user clicks a swatch, which is the one action that also destroys the colour the ring was supposed to point at.

I proved the disjointness by measurement and the rendering consequence by reading the binding and the converter. I did not capture a screenshot of the drawer — the capture harness cannot stage "a channel selected with the settings drawer open" without a device or a seeded DAQIFI_DATA_DIR, and I judged the code path unambiguous enough not to spend a build slot on it. If a reviewer wants the picture before acting, say so.

Mobile is affected differently, not equally. Views/Mobile/ChannelsMobileView.axaml:400-419 binds the same 8-entry Palette but has no selection indicator at all — no BrushColorMatch, no ring. So mobile does not show a wrong selection; it shows none, and the "your channel's colour is not in this list" half still applies.

It is upstream-inherited, confirmed against daqifi-desktop@main

Read at the default branch with gh api .../contents/...?ref=main, not from the on-disk checkout (a sibling clone's stale branch and other agents' worktrees have both been misread as upstream evidence before):

  • Daqifi.Desktop/ViewModels/ChannelsPaneViewModel.cs:92-94 — the identical 8 hexes, #4A9EFF first.
  • Daqifi.Desktop/Channel/ChannelColorManager.cs:16,95 — List<System.Windows.Media.Brush> Brushes, NewColor(), the same Material set.

So daqifi-desktop has the same defect. It is not a port regression, and it is not a parity gap either way. Since daqifi-desktop is the app being retired (#230), fixing it here and not there is the right asymmetry — no upstream filing.

What it costs, and what it does not

Deliberately testing the consequence rather than asserting the premise:

claim verdict how it was checked
The picker cannot indicate a freshly created channel's colour true disjointness measured above + the IsVisible multibinding
The auto-assigned colour is unrecoverable once the user picks a swatch true SetColor (ChannelsPaneViewModel.cs:420-425) is the only writer besides the two constructors; nothing re-reads palette 1 for an existing channel
It corrupts stored data false the persisted form is a string (DataSample.Color), written from whatever brush is current; both palettes round-trip through OxyColor.Parse fine
A channel can be left with no colour at all false AnalogChannel.cs:143 and DigitalChannel.cs:182 are the only constructors and both assign ChannelColorManager.Instance.NewColor()

Severity is cosmetic-but-user-visible: nothing breaks, the user just cannot see or re-choose the colour their channel is drawn in.

Suggested order

Each step is independently mergeable; step 1 is a decision, not code.

  1. Pick which palette wins. This is a product question and it is the whole blocker. Three options with their measured costs:
    • Auto-assign from the picker's 8. Deletes MaterialColors.cs (67 lines) and 30 of 38 palette entries — net subtraction, ring works immediately. Cost: only 8 distinct colours before repeating, against 38 today; a 16-channel device would show each colour twice.
    • Offer all 38 in the picker. Deletes the 8 inline hexes and BuildPalette. Cost: the drawer's WrapPanel grows from 8 to 38 swatches — a real layout change on both desktop and mobile, and additive in UI terms.
    • Curate one new list of ~12-16 and delete both. Most work, best result, and the only option that is a deliberate design rather than a side-effect.
  2. Collapse to the chosen palette, deleting the other. Behaviour-changing by construction (that is the point), so it wants the first tests this path has ever had — git grep -P 'ChannelColorBrush|ChannelColorManager|IBrush' -- 'Daqifi.Avalonia.Tests/***' exits 1 today, i.e. zero coverage.
  3. Fold palette 3 (MobileShellViewModel.Palette, uint[]) into the same source, so "what colour is channel i" has one answer on both heads.
  4. Only then consider whether the model should hold IBrush at all — see below. On today's evidence, do not.

Small enough to take now, independent of the decision above: Channel/AbstractChannel.cs carries a dead partial void OnChannelColorBrushChanged(Brush value) whose entire body is a comment (line 448), and ChannelColorManager's private constructor (line 74) is likewise empty-but-for-a-comment. Separately, four files alias using Brush = Avalonia.Media.IBrush; — AbstractChannel.cs:10, ChannelTileViewModel.cs:11, ChannelsPaneViewModel.cs:17, DeviceTileViewModel.cs:12 — a WPF-era alias that gives IBrush the name of a different real Avalonia type (Avalonia.Media.Brush, the abstract class, which ImmutableSolidColorBrush does not derive from). That is ~10 lines of pure subtraction plus four mechanical renames, with no behaviour change — worth folding into step 2 rather than spending a PR on alone.

The layering question I was actually sent to scope — answer: not worth doing on its own

PR #428's builder handed forward a note that Channel/IChannel.cs, Channel/DataSample.cs and Channel/ChannelColorManager.cs "carry Avalonia.Media brush types in the model layer", calling it "a genuine inversion … and pervasive". I re-derived the extent and tested the cost. The premise is true and the "pervasive" is not, and I could not demonstrate any cost.

True extent — three declaring members, not a pervasive type:

  • IChannel.ChannelColorBrush → IBrush (IChannel.cs:53) — the one interface member.
  • ChannelColorManager.Brushes → List<IBrush> and NewColor() → IBrush. Brushes is public with no consumer outside its own class (git grep -P '\.Brushes\b' finds only its own @port comment line).
  • AbstractChannel._channelColorBrush ([ObservableProperty]) plus the dead partial hook.

Non-view references, 9 lines in total: AnalogChannel.cs:155 and DigitalChannel.cs:194 (assign), DataSample.cs:127,129 (read → stringify), ChannelsPaneViewModel.cs:424 (assign), SdCardSessionImporter.cs:947,948,961,962 (take a brush from the manager, stringify it). Plus 6 XAML bindings (ChannelsPanePrototype.axaml:177,560,630, LiveGraphPane.axaml:598, ChannelsMobileView.axaml:115,352), which are a view binding a brush and are correct. Plus 0 test references — git grep -P 'ChannelColorBrush|IBrush|SolidColorBrush|ChannelColorManager|Avalonia\.Media' -- 'Daqifi.Avalonia.Tests/***' exits 1, against many lines for the same pattern over Daqifi.Avalonia/***. Searched as declarations and usages, not by filename, and including the target-typed = new( form (git grep -P '=\s*new\s*\(' over Channel/ and Loggers/ returns 20 lines, none brush-typed).

Cost, tested rather than assumed:

candidate cost verdict
Forces a UI package reference into a project that would not otherwise need one No. There is one library project in the repo (Daqifi.Avalonia.csproj); Channel/ and View/ are both inside it, and it references Avalonia for the views regardless. There is no model project to keep clean.
Blocks testing the model without a UI toolkit No, measured. AnalogChannel's constructor calls NewColor(), which builds all 38 ImmutableSolidColorBrush via Color.Parse. Daqifi.Avalonia.Tests/Device/NonFiniteStreamedReadingTests.cs constructs those channels at a dozen call sites, and dotnet test runs in the desktop job — runs-on: ubuntu-latest (.github/workflows/build.yml:94, test step at :215) — with no windowing platform, no workload and no headless-Avalonia package. That job is green on today's main (4716ce1, run 35495684952, all six jobs success).
Blocks sharing the model with another head or with Core No. All three heads are Avalonia. And Core 1.8.0 has no colour concept to share with — see below.
Already causes a concrete bug Not the type; the palette split does. That is this issue.

Does Daqifi.Core already offer the right shape? No. Read PublicAPI.Shipped.txt (2669 lines) and PublicAPI.Unshipped.txt (64 lines) at tag v1.8.0, the pinned version, via gh api .../contents/...?ref=v1.8.0. Grepping colou?r|brush|palette|rgb|hex|argb across both returns only firmware hex-file APIs — zero colour surface. Positive control on the same files: Daqifi.Core.Channel.AnalogChannel returns 31 lines, none of them a colour. I am not proposing a Core issue. A channel's display colour is a presentation choice made by whichever app draws it; pushing it into a hardware library would be the wrong direction, and Core's IChannel carrying a colour would have no consumer but this app.

Conclusion on the inversion: it is one interface member and a palette class, in a single-project app, in the only project that could ever hold them, exercised headlessly in CI. It is a purity concern with no measured cost today. Changing IBrush to Color would be a mechanical rename that touches ~8 sites and buys nothing on its own — and it would be net-additive if it grew a converter layer to keep the XAML bindings working. Do not open a PR for it. If the palette work above lands, the type question becomes a one-line afterthought inside it; if it does not, leave it alone.

flowchart LR
  subgraph owners["three answers to 'what colour is this channel?'"]
    A["ChannelColorManager<br/>38 Material IBrush<br/><i>model layer</i>"]
    B["ChannelsPaneViewModel.Palette<br/>8 IBrush<br/><i>view-model layer</i>"]
    C["MobileShellViewModel.Palette<br/>8 uint<br/><i>mobile shell</i>"]
  end
  A -->|"NewColor() — every new channel"| C1["IChannel.ChannelColorBrush"]
  B -->|"user clicks a swatch — SetColor"| C1
  C1 -->|".Color.ToString()"| S["DataSample.Color (string, persisted)"]
  S -->|"ChannelSeriesColor.ParseOrFallback"| O["OxyColor — the plotted line"]
  C -->|"by index"| M["mobile LivePlot series"]
  A -. "disjoint — the ring never matches" .-> B
Loading

Open questions I could not settle

  1. Which palette is the intended one? Nothing in either repo's history or comments says. The 38 Material colours look like the original design; the 8 look like the prototype redesign that View/Prototype/ is named for. Needs a human.
  2. Was the 8-swatch drawer ever meant to be the complete set, or a "recent/favourites" strip? If the latter, the fix is a different shape again (show the current colour alongside the eight).
  3. Should the mobile uint[] plot palette track the channel's own colour at all? Today a mobile channel's chip colour and its plotted line colour are independent by construction, which may be deliberate.
  4. Is ChannelColorManager._colorCount++ meant to be thread-safe? NewColor() is called from channel construction and from SdCardSessionImporter background work; the non-atomic increment can hand two channels the same colour under a race. Low impact and not what this issue is about, but I did not find it documented either way.

Cross-references

Line numbers are against main at 4716ce1.

Activity

  1. tylerkron commented on Sep 20, 2026

    @tylerkron
    ContributorAuthor

    A deliberate enumeration of desktop/mobile divergence turned up a third channel palette, disjoint from both of the two this issue names: MobileShellViewModel.Palette (8 Material 300 shades) colours the mobile live plot by selection index rather than by the channel's own colour. Filed as #433. Worth deciding the two together — as things stand the app holds three mutually disjoint channel palettes, and #433's suggested fix (colour from ChannelColorBrush) removes one rather than adding a fourth.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions