Skip to content

Fix squared/unicolor mean dispatch in both process overloads (fix #2057) - #2058

Merged
Lord-Grey merged 2 commits into
hyperion-project:masterfrom
Kirorus:fix/image-processor-mean-mapping
Oct 8, 2026
Merged

Lord-Grey merged 2 commits into
hyperion-project:masterfrom
Kirorus:fix/image-processor-mean-mapping

Conversation

@Kirorus

@Kirorus Kirorus commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #2057.

mappingTypeToInt() and mappingTypeToStr() identify ID 1 as multicolor_mean_squared and ID 2 as unicolor_mean, but both process() template overloads call the opposite algorithms.

This PR corrects the four algorithm calls:

  • ID 1 uses getMeanSqrtLedColor() (per-LED RMS).
  • ID 2 uses getUniLedColor() (whole-image arithmetic mean repeated on all LEDs).
  • ID 0 and dominant-color modes are unchanged.

Mapping names, numeric IDs, JSON schemas, API conversions, LED coordinates, black-border processing, and pixel averaging implementations are not changed.

Adds a small Python standard-library source-contract regression test covering all seven names, their stable IDs/reverse conversions, and the algorithm selections in both overloads. It intentionally does not claim to test pixel arithmetic or instantiate a native Qt daemon.

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Build-related changes
  • Other, please describe:

If changing the UI of web configuration, please provide the before/after screenshot:

Not applicable: no UI change.

Does this PR introduce a breaking change?

  • Yes
  • No

No configuration schema or public API change. The two affected selections now perform their advertised algorithms. Users who intentionally selected the opposite label as a workaround will need to choose the correctly named mode after updating.

The PR fulfills these requirements:

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature
  • Related documents have been updated (docs/docs/en)
  • Related tests have been updated

Not applicable: this is a dispatch bugfix, not a new feature.

PLEASE DON'T FORGET TO ADD YOUR CHANGES TO CHANGELOG.MD

  • Yes, CHANGELOG.md is also updated

Other information:

Verification

  • python3 test/test_image_processor_mapping.py -v: 2 tests pass, with algorithm checks for all seven named modes in both overloads.
  • The same test against the unmodified master implementation fails for both multicolor_mean_squared and unicolor_mean; stable name/ID round-trips pass. This establishes that the regression test catches the original mismatch.
  • git diff --check: passes.
  • Supplemental local C++ dispatch probe: the original name-conversion methods and both process template bodies were extracted unchanged and compiled with c++ -std=c++17 -Wall -Wextra -Werror -O2, using test doubles for Qt containers/logging, setup/border handling, and LED routines that report which algorithm was invoked. 26 checks / 4 failures on unmodified master; 26 checks / 0 failures with this fix. This is only dispatch validation, not a full Hyperion build or RGB calculation test.

The standalone source-contract test can be run with Python 3 from the repository root using the command above; it is not wired into the existing native test runner.

Validation limits

A full native Qt/Hyperion build was not run locally because the review host lacks CMake and Qt development packages. Repository CI and maintainer review are still needed. The patched daemon has not been installed or tested against real LED hardware. The live 2.2.1 observation and ordinary-mean workaround are documented separately in #2057; there is no claim of an already-deployed fix.

Correct the squared multicolor and unicolor algorithm selections without changing mapping names or IDs. Add source-contract regression coverage for both overloads and all named modes. Independently reviewed before publication.
@hyperion-project

Copy link
Copy Markdown

Hello @Kirorus 👋

I'm the Hyperion Project Bot and I want to thank you for
contributing to Hyperion with your pull requests!

To help you and other users test your pull requests faster,
I'll create a link for you to your workflow artifacts.

🔗 https://github.com/hyperion-project/hyperion.ng/actions/runs/37353313393

Of course, if you make changes to your PR, I will create a new link.

Best regards,
Hyperion Project

@hyperion-project

Copy link
Copy Markdown

Hey @Kirorus I created a new link to your workflow artifacts:
🔗 https://github.com/hyperion-project/hyperion.ng/actions/runs/37831889393

@Lord-Grey

Lord-Grey commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

@Kirorus Thank you for sharing your finding and already providing a fix via a PR.
This is very much appreciated.

PS: I used the opportunity to fix the tracing category in the file too :). Hope that is OK for you.

@Lord-Grey Lord-Grey self-assigned this Oct 8, 2026
@Lord-Grey Lord-Grey added the Bug label Oct 8, 2026
@Lord-Grey Lord-Grey added this to Core Oct 8, 2026
@Lord-Grey
Lord-Grey self-requested a review October 8, 2026 19:31
@Lord-Grey
Lord-Grey merged commit 9a73e82 into hyperion-project:master Oct 8, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

multicolor_mean_squared and unicolor_mean dispatch to swapped algorithms

2 participants