Skip to content

Report pump max_speed attribute as a scalar, not a 1-tuple - #69

Open
luscoma wants to merge 1 commit into
Crewski:mainfrom
luscoma:fix/pump-max-speed-scalar
Open

luscoma wants to merge 1 commit into
Crewski:mainfrom
luscoma:fix/pump-max-speed-scalar

Conversation

@luscoma

@luscoma luscoma commented Sep 13, 2026

Copy link
Copy Markdown

While investigating some issues with service mode toggling (PR incoming), claude saw an issue where the max_speed tuple was incorrectly initialized as an array. I doubt anyone ever depended on this but in the interest of fixing a simple 1-liner I figured I'd cut this into a branch to send it.

Basically just fixes a incorrect tuple so the max speed as reported by njsPC is correct initialized as a scalar.

Claude's wordy but detailed summary :)

PumpSpeedSensor wrapped the maxSpeed value in a tuple because of a stray trailing comma, so the max_speed state attribute was published as a one-element sequence instead of a number:

self._state_attributes["max_speed"] = (pump["maxSpeed"],)

Home Assistant renders that as (2700,) in the template engine and serialises it as [2700] over the REST and websocket APIs. min_speed, set on the line immediately above, is a plain int. The asymmetry is entirely this typo: nodejs-PoolController sends both as scalars, e.g. "minSpeed": 450, "maxSpeed": 2700.

Any consumer that treats the attribute as a number breaks, in one of two ways:

{{ state_attr(..., 'max_speed') | float }}
raises, which aborts the entire template render

{{ state_attr(..., 'max_speed') | float(0) }}
silently evaluates to 0

The second is the damaging case, because | float(0) is the defensive form the Home Assistant documentation recommends and it fails without any error. Verified against a live IntelliFlo VS reporting 2700 RPM with max_speed 2700:

speed >= max_speed   ->  True   (also True at 450 RPM)
max_speed - speed    ->  -2700  (correct answer: 0)

So "pump is at maximum speed" conditions are always true regardless of actual speed, and "percent of maximum" calculations divide by zero.

Dropping the trailing comma makes max_speed an int, matching min_speed and the values the controller actually sends. No other behaviour changes.

Present since df526a7 (2023-02-03, "Changes to device based objects"), where the tuple form was introduced with the class; the attribute has never been a scalar. PumpFlowSensor's min_flow/max_flow were already correct, and this is the only occurrence of the pattern in the integration.

Note for anyone upgrading: automations or templates that worked around this by indexing the attribute (for example | first, [0], or | float(0) comparisons tuned to the broken value) will need updating, as max_speed is now a bare number.

PumpSpeedSensor wrapped the maxSpeed value in a tuple because of a stray
trailing comma, so the max_speed state attribute was published as a
one-element sequence instead of a number:

    self._state_attributes["max_speed"] = (pump["maxSpeed"],)

Home Assistant renders that as (2700,) in the template engine and
serialises it as [2700] over the REST and websocket APIs. min_speed, set
on the line immediately above, is a plain int. The asymmetry is entirely
this typo: nodejs-PoolController sends both as scalars, e.g.
"minSpeed": 450, "maxSpeed": 2700.

Any consumer that treats the attribute as a number breaks, in one of two
ways:

  {{ state_attr(..., 'max_speed') | float }}
      raises, which aborts the entire template render

  {{ state_attr(..., 'max_speed') | float(0) }}
      silently evaluates to 0

The second is the damaging case, because | float(0) is the defensive form
the Home Assistant documentation recommends and it fails without any
error. Verified against a live IntelliFlo VS reporting 2700 RPM with
max_speed 2700:

    speed >= max_speed   ->  True   (also True at 450 RPM)
    max_speed - speed    ->  -2700  (correct answer: 0)

So "pump is at maximum speed" conditions are always true regardless of
actual speed, and "percent of maximum" calculations divide by zero.

Dropping the trailing comma makes max_speed an int, matching min_speed
and the values the controller actually sends. No other behaviour changes.

Present since df526a7 (2023-02-03, "Changes to device based objects"),
where the tuple form was introduced with the class; the attribute has
never been a scalar. PumpFlowSensor's min_flow/max_flow were already
correct, and this is the only occurrence of the pattern in the
integration.

Note for anyone upgrading: automations or templates that worked around
this by indexing the attribute (for example | first, [0], or
| float(0) comparisons tuned to the broken value) will need updating, as
max_speed is now a bare number.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant