XKNX / XKNX/xknxtoolkit

Verify whether HSV color parameters need RGB->HSV conversion or are a direct passthrough

Open
#61 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
4
Forks
0
Avg merge
15h 38m
Merged PRs (30d)
37

Description

Background

_encode_color in packages/product/src/xknxmono/product/parser_v2/encode.py parses a Color parameter's value as a "#rrggbb"-style hex string. For ColorSpace="RGB", the three byte-pairs are written directly as R, G, B. For ColorSpace="HSV", the current code instead treats those same three byte-pairs as literal RGB and runs them through _rgb_to_hsv (a standard RGB->HSV conversion formula) before writing the result as H, S, V.

The open question

There is no KNX DPT for HSV at all - confirmed absent from both the KNX Standard's datapoint type definitions and calimero-core's DPT translators. So unlike RGB (DPT 232.600, a direct 3-octet match) and RGBW (DPT 251.600, though that turned out to describe the bus telegram, not the stored parameter - see #60), there's no independent wire-format reference to check HSV against at all.

The two live possibilities:

  1. Conversion happens: the hex string is always a literal RGB color regardless of ColorSpace, and an HSV-space parameter's stored/downloaded bytes are the converted H, S, V values - what the code does today.
  2. Direct passthrough: the hex string's three byte-pairs already mean H, S, V directly for an HSV-space parameter (the value never was RGB to begin with - the color-picker UI just reuses the same generic 3-byte hex-string storage format for both spaces, the same way DPT-style stored values often reuse a common wire shape across sub-types), and no conversion should happen at all - matching how RGB and RGBW are both direct byte-for-byte passthroughs with no computation involved.

Nothing pins this down conclusively either way from what's been checked so far. If (2) is correct, _rgb_to_hsv is not just unverified but actively wrong - it would silently corrupt a color that was already the right H, S, V values by running it through an RGB interpretation it was never meant to have.

What would settle it

  • A real HSV-typed parameter's value string from an actual product, ideally compared against what a device/ETS actually downloads for it.
  • Any authoritative reference to the exact ColorSpace="HSV" value/encoding convention (this hasn't turned up in what's publicly available so far).

If it turns out to be a direct passthrough

Replace _rgb_to_hsv and the branch that calls it in _encode_color with the same direct byte passthrough already used for RGB (parse the 3 hex byte-pairs, write them verbatim) - RGB and HSV would then be identical code paths, differing only in which display widget ETS shows for editing them.

Current state

Left as the existing RGB->HSV conversion for now, since removing it would just trade one unverified guess for another without new evidence. _rgb_to_hsv's docstring already flags the formula as a standard-but-unverified choice.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in packages/product/src/xknxmono/product/parser_v2/encode.py, focusing on _encode_color and _rgb_to_hsv. Find an actual HSV-typed product value and compare it with device or ETS downloads, or locate an authoritative ColorSpace="HSV" encoding reference. Done means the convention is established and the implementation is aligned with it.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.