Conversation
5109d35 to
e587c41
Compare
flip-dots
left a comment
flip-dots
left a comment
There was a problem hiding this comment.
The more I read this PR the more I think this needs to be in its own dedicated class (maybe a new type e.g cloud_device.py) since its quite different from the way a lot of other devices work but I am not entirely decided about that.
Is this cloud stuff in the negotiations 100% needed to make this device work on the latest firmware? How does the app handle local usage only? Is there a reset command we can use to use our own cloud ID or some sort of adoption process we can use?
I am not completely against allowing this library to work with a device already set up with the cloud (especially for the solar banks for which its more or less mandatory) but if it can be avoided (which I am hoping it can be) I would rather we make sure there is nothing that can be done there first before implementing this cloud protocol that is overly difficult for an end user to use.
| Use this class to connect, monitor and control a Gen 2 C2000(X) power | ||
| station. This model is also known as the A1783. | ||
|
|
||
| The C2000 G2 is the larger sibling of the C1000 G2 (A1763) and shares its |
There was a problem hiding this comment.
Technical details like this (anything which starts talking about implementation rather than what you need to know to use the module) which are automatically rendered in the docs should be hidden behind a collapsed note like here
| """ | ||
| return int(time.time()).to_bytes(4, "little").hex() | ||
|
|
||
| @staticmethod |
There was a problem hiding this comment.
This code sucks but I have had a quick look around and there does not seem to be a better way to do it. Maybe create a utils.py file and hide this code inside it, device.py is already large/complicated enough.
There was a problem hiding this comment.
I shouldn’t be surprised that even in 2026 time zones are still awful to deal with.
| def summary(self) -> dict[str, object]: | ||
| """Fields from the latest protobuf device-summary frame, if any. | ||
|
|
||
| Populated from a ``_PROTOBUF_TELEMETRY_COMMANDS`` frame (e.g. the C2000 G2's |
There was a problem hiding this comment.
This is probably fine for now but I might end up moving this stuff into a dedicated class which supporting devices can implement to get this functionality depending upon how widespread this bonus telemetry ends up being.
| return await self._client.write_gatt_char( | ||
| UUID_COMMAND, bytes.fromhex(NEGOTIATION_COMMAND_1) | ||
| UUID_COMMAND, | ||
| self._negotiation_packet("0003", "a30120a40200f0"), |
There was a problem hiding this comment.
Use constants for these and other magic values like them.
| #: (SolixBLE #22) firmware rejects a plain ``a224<uuid>`` registration (ack status | ||
| #: ``09``) and only arms telemetry once the registration carries the account that | ||
| #: owns the device. Account-specific -- MUST be sourced from config, never hard-coded | ||
| #: upstream. ``None`` falls back to the legacy ``a224<uuid>`` payload. |
There was a problem hiding this comment.
Do we know what happens if the device is setup with Bluetooth only (i.e not setup using the cloud)?
It would be good to have a way to use these newer devices without the need for the cloud at all.
That being said I think it still makes sense to support this for devices like the Solarbank where you might want both cloud and local control.
There was a problem hiding this comment.
Do we know what happens if the device is setup with Bluetooth only (i.e not setup using the cloud)?
Now we do — I unsealed two A2345s and tested it. The owner_user_id path fails silently on an unowned device, but there is a cloud-free path, and it's the same opcode.
Never bound to any account: 4027 carrying a 40-byte a2 = account id gets no reply at all. Not 09, not 04 — nothing. The session stays open, 4200/420a/420b go out, connect() reports success, and the device simply never streams, dropping the link ~40 s later. Indistinguishable from a missing keepalive or a flaky link — and it is exactly what #22 looks like (NEGOTIATED: True, every field -1).
The cloud-free path: a 16-byte token in field 2 instead of the 40-byte account id.
4027 field 2 |
response | then |
|---|---|---|
| 40 B account id, device owned by that account | 00 |
streams (this stack — confirmed by @pkolbus on #17's hardware) |
| 40 B account id, device owned by nobody | silence | never streams |
| 16 B token, new to the device | 09 + a1 02 1e00 (30) |
press the side button → device pushes an unsolicited 4827 00 → streams |
| 16 B token, already enrolled | 00 |
streams, no button |
The 09 is not a refusal — it means awaiting physical confirmation. The session stays up and the grant arrives asynchronously 5–8 s later when the button is pressed. It persists: reconnecting with the same token gets 00 immediately thereafter.
The token content is not checked against the device — I enrolled one unit using the other unit's serial, then ABCDEFGHIJKLMNO. But it is not ignored either: each new token needs its own button press, including on an already-paired unit. So it is a real pairing model — one press per client, and enrolling one client does not open the device to the next.
Reproduced identically on both units (AQLQJB1G22202385, AQLQJB1G22202305), so this is model behaviour, not a per-unit quirk.
Inference: field 2's length selects the mechanism — 0x28 (40) = account binding, 0x10 (16) = local client token. That fits your point about still wanting cloud support for devices like the Solarbank: both live on the same opcode, and a device can support either.
One library gap this exposes. The grant arrives on pattern 030101 — neither 030001 nor 03010f — so the library logs
Unexpected packet type b'\x03\x01\x01' sent by device!
and discards it, which is the single most important frame in the exchange. Handling 030101, and surfacing the 09/1e00 state so a UI can prompt "press the button on the device", is the minimum needed for cloud-free setup to work at all.
There was a problem hiding this comment.
I didn't realize the mistake, but ABCDEFGHIJKLMNO is only 15, not 16, so it even accepted a non-well-formed/truncated TLV.
| """ | ||
| Send the negotiation initiation command. | ||
| """ | ||
| def _live_negotiation_packet(self, cmd: str, extra: str = "") -> bytes: |
There was a problem hiding this comment.
Why not reuse _negotiation_packet() from device.py?
There was a problem hiding this comment.
No good reason — I missed that device.py already had _negotiation_packet(). The two are identical apart from wrapping the payload in _encrypt_payload, so it wants a hook on the base rather than a near-duplicate.
Worth flagging where it lives, though: this line isn't #50's. It's from #49 (pr/negotiations), which #50 was stacked on when you reviewed it — I can't open a PR whose base is another of my branches in your repo, so the diff carried #49's changes along. Since the rebase, #50 doesn't touch prime_device.py at all.
Which makes the fix a question rather than a promise: given you're planning to merge #61 instead of #48 and build the dynamic timestamp/timezone work on top of it, that whole method may be replaced anyway. Happy to unify it in #49 if #49 still has a future, or to leave it alone if #61 subsumes it — your call, and I'd rather not spend your review time on code you're about to delete.
| # Device identity (chip, BLE firmware, serial, MAC) is sent only | ||
| # in this stage-3 message, never in telemetry, so persist it for | ||
| # the identity properties to read. | ||
| self._device_info = parameters |
There was a problem hiding this comment.
I think self._device_info and self._summary should be named self._data_device and self._data_summary to be consistent with the existing self._data and distinguish it from other cached values.
The Anker manufacturer record (company id 0xffff) carries the device MAC, model, and a capability byte declaring which negotiation path the device accepts -- readable at scan time, before any frame is sent. Add a parser for it as the basis for choosing the cleartext vs encrypted handshake per device rather than by product class. Capability is length-relative (last byte when present, absent on the F3800), so it is derived from the sku length rather than read at a fixed offset. Decoded against the app's own field values for five bench records. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ation Reads the advertised capability byte to pick the negotiation path, and adds the encrypted GCM (4xxx) handshake plus client-token authorization to the base class, built to the A1783 comms-module firmware. A device whose advert sets the ECDH capability bit (or a class that defaults to it) negotiates under the static GCM key, echoes the device's own auth mode in 4005, carries a signed int32 UTC offset in the 4022 confer, and authorizes the link with a 4027 registration of a stable, generated client token. On hardened firmware a fresh token is accepted after a physical button press, whose grant arrives unsolicited on pattern 030101; an already-registered token authorizes immediately. `negotiated` gates on that authorization for the encrypted path. Cleartext devices are unchanged (the branch is a no-op when the capability bit is clear), and PrimeDevice keeps its own negotiation, so its captured vectors and telemetry are untouched. The static key, nonce and AAD are the values the module firmware derives from two DROM constants at connect time; the client token replaces the account owner-id binding, since the device stores it as an opaque enrollable handle and needs no cloud account. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds a usage page covering how the advertised capability byte selects the cleartext versus encrypted handshake, and how to pair a client with a stable, persisted token -- including the one-time physical button press that firmware enforcing pairing requires on the first connection. Links it into the toctree. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Rebased onto |
Harvey noted device.py is already large/complicated. The _offset_seconds_west helper (UTC offset as a signed int32 LE, seconds west, sent as a3 in the 4022 timezone confer) uses no instance state, so move it to SolixBLE/utilities.py next to the existing get_posix_tz timezone helper and import it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Harvey asked for constants over the magic values in the negotiation. Extract the two the firmware decode gives clear meaning: the client's MTU proposal (a4 of 0003/0005 -- u16 LE 0x00f0 = 61440 = "no limit", device streams at min(this, its ceiling)) and the encryptMethod it confirms (a5 of 0005 -- the device selects ECDH on a5 & 0x44). The a3 = 0x20 field stays inline: the comms- module firmware logs and ignores it, so a name would imply meaning it does not have. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reintroduces the protobuf walker (parsing.py) that the packet-layer rewrite removed, and the base-class hooks that route a protobuf device-summary frame (the C2000 G2's c490) into a `.path`-keyed `summary` map rather than the flat TLV the other telemetry frames use. The frame's protobuf blob is sliced out of the outer a2 field before walking. No device enables it yet. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two independent things, the first of which the second depends on. fix: C1000G2._post_connect and MagGo3in1._post_connect both still call _send_command(cmd=..., payload=...), but _send_command now takes a required `parameters` argument and no `payload`. Both raise TypeError on every connection. For the Gen 2 that means the subscribe command never goes out and the device streams no telemetry at all; there was no test covering _post_connect on any device, which is why it went unnoticed. Both are switched to the parameters interface and both now have a regression test asserting the exact bytes. feat: adds C2000G2 (A1783), the larger sibling of the C1000 G2. It shares the Gen 2 framing, TLV map and AC/DC control, so ports and power come from C1000G2 unchanged. On top it adds the parts of the frame that were not decoded when C1000G2 was written: - 4103 system group: display switch, brightness and timeout, plus the SoC charge cap and discharge floor. The firmware accepts any percentage, not just the app's menu -- 99 was set over BLE and read back verbatim -- so the setters range-check rather than restrict. - AC and DC output auto-off countdowns, settable and readable. These are a u32 under type 03 -- the same 4-byte form the `fe` timestamp takes, and not the u16 the display timeout uses -- so the value width is keyed off the type code rather than assumed. - 4057 realtime latch. This is the device's actual "start streaming" switch; 4100 is a one-shot poll despite its name. It is only honoured with routing byte 0x21 -- the MQTT-side 0x22 is accepted and silently dropped. The disable carries a warning: it also gates the device's periodic protobuf summary, and re-arming does not bring that back, so it must never be called from teardown. - a3[0] work status and a5[1] charge/discharge status. These are two different fields: the second trips on any flow, the first waits for a threshold, so they disagree at low load. Both test frames are real, and the attached one captures that disagreement at 2 W. - a3[0] also takes a fourth value during a firmware update that is not in the status enum, surfaced as `firmware_updating` rather than silently mapping to UNKNOWN. - a6[6:8] time remaining, bidirectional and in deci-hours. - the rest of the a4 settings block: AC input limit and frequency, AC and 12 V DC output modes, device idle timeout, ultrafast-charge and port-memory switches. - f9 version block: seven little-endian quads, exposed per submodule. The inverter slot reads zero whenever the inverter is not energised, which means idle rather than absent. - c0 expansion block for the BP2000. It is emitted whether or not a pack is attached -- absent, the fields are padded with sentinels (a 239 temperature, a 0 percentage) -- so every expansion property gates on subPackageConnectionStatus rather than on the tag being present. The block is also variable width: the serial is 16 bytes absent and 17 attached, so both length prefixes are walked instead of assuming offsets. Telemetry tests use two real decrypted c421 frames from an A1783, one before the BP2000 was attached and one after, so the absent and attached layouts are both covered. The two frames also differ in their device idle timeout, which pins that field independently of the layout. The support matrix gains a C2000 G2 column. Its cells are realigned because the table padded to visual width, which had drifted the emoji columns out of alignment with the separator row. Not included: Total Power In, which the Gen 2 frame carries only as separate AC and DC inputs with no grand total. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Enables the c490 protobuf device-summary on the C2000 G2 -- its telemetry set gains c490 and _PROTOBUF_TELEMETRY_COMMANDS routes it into `summary`. The frame's trailing a3 schema name (charging_pps_series_c_NNNN) versions the protobuf layout, so the field decoding is only correct for one revision. The device now records that schema (exposed as `summary_schema`) and warns when a unit posts an older revision (the 2025 _0002, an old firmware) or one newer than the validated _0005, whose values are then unverified. Adds tests for the a3 extraction, the summary routing, and the schema guard. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add the four fields the C2000 G2 exposes only through the cloud-armed c490 device-summary -- Max charge power, Pack voltage, Cumulative energy out and Charge presence -- to the power-station table, plus a marks legend and a note explaining they are carried in the raw summary map rather than decoded into named properties. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move the C2000 G2 class docstring's implementation detail (the shared Gen 2 stack, the added 4103/a3/f9/c0 blocks, the c490 summary) into a collapsible note, keeping the user-facing description at the top. Rename the cached protobuf-summary attributes _summary / _summary_schema to _data_summary / _data_summary_schema, consistent with the existing _data. The public summary / summary_schema properties are unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Read the hardware max input power (a3), the a6 mirror of the main battery SoC, and the AC input port status, and add set_ac_charging_power() to drive the a4 charge limit under 4101. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2 participants
Split out of #45 per review — the c490 protobuf device-summary decode.
Adds
walk_protobuf(parsing.py) +_protobuf_bodyto decode the protobuf device-summary frame (the C2000 G2'sc490) into a.pathfield map exposed viasummary, rather than mis-reading it as the 1-byte-tag TLV the base parser expects (which garbles the varint tags). Adds the C2000 G2 (A1783) device on top of the C1000 G2, and the documentation for it.Rebased onto current
mainand decoupled from #49 — this PR no longer carries the negotiation work. It is now just #48 (reassembly) + the c490 decode + its docs, three commits. The reassembly commit is included because the c490 frame is multi-fragment and does not decode without it; everything else is this PR's own.Per the review note on #52, the C2000 G2 documentation now lives here rather than in a separate docs PR:
docs/source/c2000g2.rst, itsapi.rsttoctree entry, and a C2000 G2 column in the power station support table with the rows this decode makes readable (max charge power, pack voltage, cumulative energy out, charge presence). Only the A1783 was verified against hardware; the A1763 column reflects the shared code path.142 tests pass; Sphinx builds clean.