The suite captured a stack local of initTestCase() by reference in two
lambdas that outlive it. Once the function returned they wrote into a
stack slot reused by later frames, corrupting the running test: a
QSignalSpy reported a phantom emission, reading its recorded arguments
segfaulted, and the count changed when unrelated locals were added. Both
observed failures came from that, not from the plugin. The two flags are
members now.
Covers the two remaining claims the offline suites cannot reach: a meter
slave ID of 0 opens no second connection against a real device, and
losing the meter connection leaves the parent connected and the inverter
data flowing, marking only the meter child disconnected.
That last test drives one explicit block read first. This bench replaces
the plugin timer with a null object, on purpose, so nothing would
otherwise ever mark a child connected and the cut would prove nothing.
Run against the bench simulator: 8 tests, 8 green. Each of the two
corrections was confirmed able to fail by reintroducing its pre-F3 form.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The whole point of keeping the meter connection in a hash of its own was
that not a line of the generic and solarEdge vendors would change.
Checks it rather than claiming it: a stored solarEdge connection, whose
behaviour no suite covered until now, must still end up with exactly one
connection, as must a generic one.
Also records the connection a discovery came from in
processDiscoveryResult() rather than in the discoveryFinished handler.
The initial setup calls it from a second handler on the same signal, so
recording it there made the orphan sweep depend on the order Qt happens
to invoke the two.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two most certain defects of the multi unit ID work are invisible to
the offline suites, because they only appear once a discovery has
actually run: the orphan sweep deleting the children of the other
connection, and the aggregation of the connected state.
This suite mounts the plugin behind a real ThingManager against a real
device. It is deliberately not a testcase, so "make check" never runs
it, but it is still built so it cannot rot unnoticed. Without
SUNSPEC_TEST_HOST every test skips, and no address is written into the
repository: the address of a real installation is customer data.
NOT YET RUN against hardware. It needs the address of a GEN24 and, given
the polling rate guidance, a deliberate decision to point it at a
customer inverter.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
processDiscoveryResult() makes child things disappear when their model
is no longer announced. It used to confront them with the models of the
one connection that had just finished discovering, which is only half of
the picture on a thing carrying two: the inverter discovery would delete
the meter child, and the meter discovery the inverter and storage ones,
on every single reconnect.
The sweep now waits for every connection of the parent to have reported
a successful discovery, then confronts the children with the union of
their models. A grace period releases it if a connection never reports,
so a hung discovery cannot freeze the cleanup for good.
That grace period is inferred, not measured: it is set generously
because sweeping too late leaves a stale child showing as disconnected,
while sweeping too early deletes a thing of the user. Recorded as D-2 in
DIVERGENCES.md with what to measure to replace it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A GEN24 answers for the inverter on unit ID 1 and for its Smart Meter on
unit ID 200 by default, behind the same host and port. One thing now
carries two SunSpecConnection objects, the second one driven by the
meterSlaveId parameter that until now no line of C++ ever read.
The meter connection is created during setup but left idle: it is only
brought up once the primary connection has finished discovering, so the
two never handshake at the same time towards the same inverter. An
absent meter never fails the pairing, since a GEN24 without a Smart
Meter is a legitimate installation and the parameter has a non zero
default the user did not choose.
thingRemoved() and the reconfigure path now dispose of both connections
through a single entry point. Their else if chain used to stop at the
primary one, which left the meter connection outliving its thing with an
open socket and signals pointing at a destroyed Thing.
Child things look their model up across every connection of their
parent. The two connections of a GEN24 never announce the same model ID,
so no discriminator has to be persisted.
Adds a test suite instantiating the plugin behind a real ThingManager
with two connections. It lives in its own binary because the F1 suite
checks global call counters that extra configured things would falsify.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Takes the slave ID as an argument instead of reading it from the thing,
and takes whether it is the primary connection of that thing. Only the
primary one follows the network monitor, writes the connected state of
the thing and lands in m_sunSpecConnections; a secondary one lands in a
hash of its own.
Child things are updated per connection rather than per parent, resolved
from the model each child is bound to. With a single connection that is
every child of the parent, so nothing changes here yet.
No caller creates a secondary connection at this point: behaviour is
unchanged for all three vendors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A GEN24 answers for the inverter on one unit ID and for the Smart Meter
on another, behind the same host and port. Isolate the decision to open
that second connection as a pure function, so it can be pinned down
without nymea, and leave the logging of the refusal to the caller.
Refuses a meter slave ID out of the 0..247 range, treats 0 as "no meter
on this connection" as documented, and refuses a meter slave ID equal to
the inverter one: a second socket on the same unit ID would rediscover
the very same models and duplicate every child thing. That last case is
reachable, since the discovery writes the scanned slave ID into the
thing parameter.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AC-7 asks for the meterSlaveId parameter to be advanced. That property
does not exist on nymea 1.15.2 param types: the metadata verifier
rejects unknown fields, so adding it makes the whole plugin fail to
load rather than merely hiding a parameter.
Measured by loading the plugin JSON, with and without the flag, into a
PluginMetadata linked against the installed libnymea, rather than read
from an upstream tree that may differ from the package.
Also records the three other divergences the multi unit ID work runs
into, two of which predate it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
setupThing() only knew about sunspecConnection and solarEdgeConnection, so a
froniusConnection thing fell into the final Q_ASSERT_X. In a release build the
assert is compiled out, setupThing() returns without calling info->finish(),
and the pairing hangs until the setup times out - with no message at all. In a
debug build nymead dies on the assert instead.
Take the same branch as the two vendors that already work: createConnection()
already reads the slave ID and picks the byte order for froniusConnection, so
no new logic is needed.
postSetupThing() and thingRemoved() are unchanged on purpose. The former only
special cases solarEdgeConnection, for the battery search a Fronius must not
run, and registers the refresh timer for every thing. The latter is keyed on
m_sunSpecConnections, which createConnection() now populates for Fronius as
well.
Makes the four cases of tst_sunspecplugin pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The existing suite only exercises free functions and the plugin JSON, so it
cannot see whether a thing class is dispatched at all. A thing class missing
from the dispatch in setupThing() falls into the final Q_ASSERT_X: in a debug
build the process dies, in a release build the assert is compiled out and
setupThing() returns without ever calling info->finish(), leaving the pairing
stuck with no message.
Split the tests into two suites. The logic one keeps its former scope and its
single dependency on libnymea-sunspec. The new one instantiates the plugin
behind the real ThingManager of libnymea-core and a faked HardwareManager, so
the dispatch of setupThing(), postSetupThing() and thingRemoved() is covered.
No network access takes place: the faked monitor is never reachable, so the
plugin never attempts a Modbus connection.
The four new cases fail: froniusConnection is not dispatched yet.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds a fronius vendor and a froniusConnection thing class alongside the
generic sunspec and solarEdge ones, following the SolarEdge mechanism but
none of its workarounds - no proprietary battery block and no PV
production computed by difference, since the GEN24 exposes model 160.
FroniusRules holds the vendor decisions as pure functions:
- discovery only keeps devices announcing Fronius as manufacturer;
- the byte order is the SunSpec default, not the forced big endian that
is specific to SolarEdge, and no endianness parameter is exposed;
- meterSlaveId defaults to 200, 0 meaning no meter on this connection;
- a hard 7 s floor on the poll interval. Polling faster can push the
inverter offline towards the vendor portal, and recovery needs an
AC+DC power cycle on site, so this is a guard in the code rather than
a configurable default;
- the registers the inverter accepts and then silently ignores are
refused before anything reaches the wire;
- the six HEMS battery modes map onto ChaGriSet, StorCtl_Mod, InWRte and
OutWRte, negative meaning charge;
- model 160 blocks are classified by IDStr, never by index: on a three
or four MPPT inverter the battery blocks are not in position 3 and 4.
Storages under a Fronius connection default revertTime to 300 s.
Setpoints now reread the scale factor before writing and read the written
registers back afterwards, reporting a mismatch through a new
controlHealthy state - the only defence against writes that are accepted
and then ignored without a Modbus exception.
Byte order and the revert timer semantics on model 124 are inferences, not
measurements; see specs/fronius-sunspec.md, H-2 and H-3.
setInOutWRteRvrtTms() existed but was never called. InOutWRte_RvrtTms is
the delay after which the inverter drops the setpoint and resumes its
native behaviour. Left at 0, a setpoint stays active indefinitely, HEMS
stopped, network lost or process crashed included - the battery stays
where the last command put it.
Add a per-thing revertTime parameter, 0..28800 s, defaulting to 0 so the
generic vendor keeps its current behaviour, and route every setpoint
through SunSpecStorageLogic::planSetpoint(). The plan orders the writes -
ChaGriSet, StorCtl_Mod, then the rates - and appends the revert timer
last, so the timer never starts counting against a setpoint that has not
been written yet.
Writes are issued sequentially rather than in parallel: the order is part
of the contract with the hardware.
onStorageBlockUpdated() fed chargingRate from wChaGra() and dischargingRate
from wDisChaGra(). Those are the gradients, in percent of WChaMax per
second, and read only on several inverters. The matching actions write
InWRte and OutWRte, in percent of WChaMax. The user wrote one quantity and
read back another.
Feed the two rate states from inWRte() and outWRte(), and keep the
gradients under names that cannot be mistaken for setpoints:
chargeGradient and dischargeGradient, read only.
enableDischarging was never reconciled either, while enableCharging was.
It stayed frozen on the last written value and drifted silently as soon as
the inverter changed StorCtl_Mod on its own. Reconcile both flags.
The mapping moves into SunSpecStorageLogic so it can be exercised on a
register dump without a running nymea.
chargingRate and dischargingRate map onto InWRte and OutWRte of model 124,
which are int16 setpoints ranging from -100.00 to +100.00 percent of
WChaMax. Declaring them as 0..100 makes a forced charge structurally
inexpressible: nymea rejects the negative action parameter before it ever
reaches the plugin.
Also fixes the default value, which was the boolean false on an int state.
One test per AC of specs/fronius-sunspec.md, named test_AC_<n>_<what>.
Definition criteria (value ranges, thing classes, defaults) are checked by
re-reading integrationpluginsunspec.json. Behaviour criteria run either on
a model 124 register dump fed to the real libnymea-sunspec model, or on the
pure logic units the plugin is about to grow.
All red at this point: sunspecstoragelogic and froniusrules do not exist
yet, so the suite does not compile. That is the expected starting state.
Vendor the sunspec plugin into this repo so the Fronius work and the
upstream storage fixes can live here. Sources only: build artefacts and
generated plugininfo headers are excluded.
libnymea-sunspec is deliberately NOT vendored - the plugin must stay
rebasable on upstream nymea, so it links against libnymea-sunspec-dev.
sunspec.pri resolves it via pkg-config, with a NYMEA_SUNSPEC_PATH
override for a local sysroot, mirroring modbus.pri.
See sunspec/VENDORED.md.
