Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Next

- **Fixed** active control silently breaking until a restart after a single invalid grid reading (a NaN value, e.g. from a briefly unavailable ESPHome sensor or a glitchy meter source): the controller could get stuck sending every battery a small constant discharge command regardless of the actual grid. An invalid reading is now treated like an unavailable meter — batteries hold their output for that poll and normal control resumes with the next good reading ([#548](https://github.com/tomquist/astrameter/issues/548), [#550](https://github.com/tomquist/astrameter/pull/550)).


## 2.2.4

Expand Down
25 changes: 21 additions & 4 deletions esphome/components/ct002/ct002.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ void CT002Component::setup() {
this->num_phases_ = (this->power_sensor_l2_ != nullptr) ? 3 : 1;

auto cache = [this](size_t i, float v) {
// NAN is ESPHome's "no data" idiom (unavailable sensor, filter gap) —
// never a reading. Skip it without refreshing the stamp so the last good
// value bridges a transient gap and a persistent outage ages out into
// the meter-unavailable path, and so NaN never reaches the other
// raw_values_ readers (Marstek MQTT / cloud reporting). Issue #548.
if (!std::isfinite(v)) return;
this->raw_values_[i] = v;
this->raw_stamp_ms_[i] = ::esphome::millis();
};
Expand Down Expand Up @@ -302,10 +308,21 @@ void CT002Component::handle_request_(const uint8_t *data, size_t len,
// active control so the stateful controller (grid-state predictor, saturation
// EMA, ...) never treats a fabricated zero grid as a fresh sample and emits a
// non-zero delta from its internal state — the wind-up issue #403 guards
// against. The battery holds on the literal zero adjustment instead. Mirrors
// ct002.py _handle_request.
const bool meter_ok = !values.empty();
if (values.empty()) values = {0.0f, 0.0f, 0.0f};
// against. The battery holds on the literal zero adjustment instead.
//
// A non-finite reading (NaN/Inf) is a meter failure too, not a sample: one
// NaN fed into the balancer poisons the grid-state predictor permanently
// (every later innovation is NaN, so no fresh meter sample can ever correct
// the estimate) and pins each battery at the ramp-pacing base step until
// reboot (issue #548). Mirrors ct002.py _handle_request.
bool meter_ok = !values.empty();
for (float v : values) {
if (!std::isfinite(v)) {
meter_ok = false;
break;
}
}
if (!meter_ok) values = {0.0f, 0.0f, 0.0f};
while (values.size() < 3) values.push_back(0.0f);
values.resize(3);

Expand Down
25 changes: 25 additions & 0 deletions src/astrameter/ct002/ct002.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,20 @@ def _bucket_for_phase(phase: str) -> str:
return "x"


def _values_finite(values) -> bool:
"""True iff every meter value coerces to a finite number.

Numeric strings are tolerated (some sources deliver them); NaN/Inf or
garbage counts as a meter failure so the handler takes the hold path
instead of feeding it to the stateful controller (issue #548).
"""
try:
return all(math.isfinite(float(v)) for v in values)
except (TypeError, ValueError, OverflowError):
# OverflowError: float(10**400) — an int too large for a float.
return False

Comment thread
tomquist marked this conversation as resolved.

# ---------------------------------------------------------------------------
# Per-consumer state
# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -1059,6 +1073,17 @@ async def _handle_request(self, data, addr, transport):
values = self._get_consumer_value(consumer_id)
if values is None:
values = [0, 0, 0]
elif not _values_finite(values):
# A non-finite reading (NaN/Inf from a flaky source or a
# filter chain fed one) is a meter failure, not a sample.
# One NaN fed into the stateful controller poisons the
# grid-state predictor permanently: every later innovation
# is NaN, so no fresh meter sample can ever correct the
# estimate, and each battery ends up pinned at the
# ramp-pacing base step until restart (issue #548). Take
# the same hold path as an unavailable meter.
meter_failed = True
values = [0, 0, 0]
raw_values = ([*list(values), 0, 0, 0])[:3]
meter_value = sum(parse_int(v, 0) for v in raw_values)
is_active = self.is_consumer_active(consumer_id)
Expand Down
60 changes: 58 additions & 2 deletions src/astrameter/ct002/ct002_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

import pytest

from astrameter.ct002.ct002 import CT002
from astrameter.ct002.protocol import build_payload
from astrameter.ct002.ct002 import CT002, _values_finite
from astrameter.ct002.protocol import build_payload, parse_request


class _RecordingTransport:
Expand Down Expand Up @@ -236,3 +236,59 @@ async def test_poll_answered_again_after_burst_coalesced() -> None:
# A later poll (a fresh reading) is still answered — no permanent drop.
await ct._handle_request(_poll(mac), addr, transport)
assert len(transport.sent) == 2


# ---------------------------------------------------------------------------
# Non-finite meter readings (issue #548): a NaN/Inf sample must take the same
# zero-delta hold path as an unavailable meter and leave the stateful
# controller untouched, so control recovers on the next finite reading.
# Before the guard, one NaN poisoned the grid-state predictor permanently
# (every later innovation is NaN) and the pace clamp turned the NaN reading
# into a constant +pace_base_step command until restart.
# ---------------------------------------------------------------------------


@pytest.mark.parametrize("bad", [float("nan"), float("inf"), float("-inf")])
async def test_non_finite_reading_holds_and_control_recovers(bad: float) -> None:
clock = FakeClock()
ct = CT002(ct_mac="", active_control=True, clock=clock)
grid = [300.0]

async def before_send(_addr, _fields=None, _consumer_id=None):
return [grid[0], 0.0, 0.0]

ct.before_send = before_send
transport = _RecordingTransport()
addr = ("192.168.178.134", 22222)

async def poll() -> list[str]:
clock.now += 15.0
await ct._handle_request(_poll("02b250b26777", power=0), addr, transport)
fields, err = parse_request(transport.sent[-1])
assert err is None
return fields

# Warm poll: import drives a positive (discharge) target.
r = await poll()
assert int(r[4]) > 0

# The meter glitches: a non-finite sample answers with a zero-delta hold.
grid[0] = bad
r = await poll()
assert [r[i] for i in (4, 5, 6, 7)] == ["0", "0", "0", "0"]

# The meter recovers with an export reading: control resumes and steers
# negative — a poisoned predictor kept this pinned at +pace_base_step.
grid[0] = -300.0
r = await poll()
assert int(r[4]) < 0


def test_values_finite_helper() -> None:
assert _values_finite([1, 2.5, "300"]) is True
assert _values_finite([]) is True
assert _values_finite([float("nan")]) is False
assert _values_finite([1.0, float("inf")]) is False
assert _values_finite(["abc"]) is False
assert _values_finite([None]) is False
assert _values_finite([10**400]) is False # float() raises OverflowError
41 changes: 41 additions & 0 deletions tests/components/ct002/test_shared_e2e.py
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,47 @@ def test_convergence(backend) -> None:
)


@pytest.mark.timeout(30, func_only=True)
def test_nan_meter_reading_holds_then_control_recovers(backend) -> None:
"""A NaN grid reading is answered with a zero-delta hold and leaves no
trace in the controller: the next finite reading steers normally.

Regression guard for issue #548: a single NaN sample used to flow into the
balancer and poison the adaptive grid-state predictor permanently (a NaN
innovation never clears the trust gate, so no later meter sample could
correct the estimate), after which the ramp-pacing clamp turned every
reading into a constant +pace_base_step discharge command until restart.
"""
mac = "ABCDEF012345"
backend.set_clock(40000)
backend.set_grid(300) # importing → discharge (+)
r = backend.poll(mac, "A", 0)
assert r is not None and int(r[4]) > 0, (
f"[{backend.name}] warm poll should drive discharge (+), got {r and r[4]}"
)

# The meter glitches: one NaN sample (ESPHome sensors publish NAN for
# "unavailable", and a filter chain fed one propagates it).
backend.advance_clock(DEDUPE_WINDOW_S + 5)
backend.set_grid(float("nan"))
r = backend.poll(mac, "A", 0)
assert r is not None, f"[{backend.name}] no response to poll during NaN reading"
assert [r[i] for i in (4, 5, 6, 7)] == ["0", "0", "0", "0"], (
f"[{backend.name}] NaN reading must take the zero-delta hold path, got {r[4:8]}"
)

# The meter recovers with an export reading: control must resume and steer
# negative — with a poisoned predictor this stayed pinned at +pace_base_step.
backend.advance_clock(DEDUPE_WINDOW_S + 5)
backend.set_grid(-300)
r = backend.poll(mac, "A", 0)
assert r is not None, f"[{backend.name}] no response after meter recovery"
assert int(r[4]) < 0, (
f"[{backend.name}] control must recover after a NaN sample: export "
f"should drive charge (-), got {r[4]}"
)


@pytest.mark.timeout(30, func_only=True)
def test_clock_gated_dedup(backend) -> None:
"""The dedup window is driven by the (mock) clock on both stacks: a repeat
Expand Down