Conversation
flip-dots
left a comment
There was a problem hiding this comment.
I am not certain but I suspect that these devices would work if you just changed the base class from PrimeDevice to SolixBLEDevice for them since the negotiations look very similar to that of earlier Solix devices and that protocol is implemented in the base SolixBLEDevice.
Other than that try to stick to pre-existing convention, like properties not being inherited, using existing types, and using the same property names.
| @property | ||
| def usb_c2_current(self) -> float: | ||
| """USB C2 Port current (A). | ||
| def usb_c1_switch(self) -> bool: |
There was a problem hiding this comment.
Is it not possible to use PortStatus like the other devices?
| @property | ||
| def usb_c2_power(self) -> float: | ||
| """USB C2 Port power (W). | ||
| def usb_c2_switch(self) -> bool: |
There was a problem hiding this comment.
Needs to use the same naming scheme as other devices. e.g here
| """ | ||
| return PortStatus(self._parse_int("a4", begin=1, end=2)) | ||
|
|
||
| @property |
There was a problem hiding this comment.
Device properties (e.g power, voltage, etc) should only be on the device class itself (e.g C300.py) not in a class inherited by others (e.g device.py / prime_device.py).
Hard coding port selections into a class which might be inherited by multiple devices prevents you from using it with future devices which may not have those ports. E.g prime_charger_160w is unable to use this class since it only has 3 USB C ports and no type A ports.
| _LOGGER = logging.getLogger(__name__) | ||
|
|
||
| #: Cleartext-negotiation / confer packet pattern (``0xxx`` and ``4022``/``4023``). | ||
| _NEGOTIATION_PATTERN = b"\x03\x00\x01" |
| class PrimeChargingStation240w(PrimeUsbCharger): | ||
| """Anker Prime Charging Station (240W / A91B2), an 8-in-1 charging station. | ||
|
|
||
| Despite sharing the Prime USB-charger telemetry layout, the station is **not** |
There was a problem hiding this comment.
Technical details not needed to simply use the library should be hidden in a collapsed note.
|
|
||
|
|
||
| class PrimeChargingStation240w(PrimeUsbCharger): | ||
| """Anker Prime Charging Station (240W / A91B2), an 8-in-1 charging station. |
There was a problem hiding this comment.
Damn all the cool Anker stuff seems to be for the US market only, it would be cool as hell if there was a UK/EU version of this.
| )[1:] | ||
|
|
||
| stages = ( | ||
| ("0001", "a104" + self._ts(), "0801"), |
There was a problem hiding this comment.
This looks suspiciously similar to the negotiation for the regular Solix devices. Does treating it as a Solix rather than a Prime type device work?
| timezone = self._local_posix_tz().encode() | ||
|
|
||
| # 4022 -- timezone; 4023 -- bind device serial (both AES-CBC, 030001). | ||
| await self._send_packet( |
There was a problem hiding this comment.
Given the similarity to the Solix style devices which usually don't need polling to provide any info, is this needed?
There was a problem hiding this comment.
You're right that the Solix-style devices stream without polling, and — as in the capability-byte thread above — the A91B2 is one of them for negotiation. Where it differs is telemetry, and this is the concrete case: its real-time feed is a latched, self-expiring stream, not a persistent push.
420b arms the 4303 stream, but the station only pushes for a bounded window. In the decompiled MCU firmware, the real-time trigger (cmd20b) loads an internal counter with 10 and decrements it per tick, so 4303 goes quiet ~8–10s after each arm. So it's not polling to fetch state (the connect-time 4a00 snapshot already does that); it's re-arming a stream that shuts itself off.
Without it, _last_packet_timestamp goes stale and any staleness-based consumer (e.g. the HA coordinator) treats the device as dropped and re-runs _post_connect — which re-sends the one-time confer (4022/4023) and re-binds the serial every cycle. _keep_alive just re-issues 4200 + 420b on its interval to keep the feed live; it deliberately does not re-send the confer (that's why connect is split into _post_connect = confer-once vs _request_stream = the re-armable 4200/420b, and the keep-alive only calls the latter).
It's the same base _keep_alive hook, and the same reason the C2000 G2 uses it: there the 4057 real-time latch can go dead for 20s+ between c421 pushes, so a 4100 re-poll bridges the gap. Same pattern here (the 420b/4303 latch lapsing, just a shorter ~8-10s window) — so it stays device-specific and off the path of the Solix devices that genuinely don't need it.
| # ------------------------------------------------- 4a00 snapshot additions | ||
|
|
||
| @property | ||
| def ac_1_switch(self) -> bool: |
There was a problem hiding this comment.
Use the same property names as existing ones from other devices. There is already ac_output and since this is the first device with two AC outputs I think it makes sense to call them ac_output_1 and ac_output_2. Try to stick to convention for the others as well since the Home Assistant integration which uses this library relies on different devices using the same names for ports.
| return bool(self._parse_int("ab", begin=1, end=2)) | ||
|
|
||
| @property | ||
| def usb_total_power_out(self) -> float: |
There was a problem hiding this comment.
If you really want this property we can keep it but I would rather avoid adding more properties that don't already exist (e.g total_power_out is already established) since its more maintenance and testing and its easy enough for someone to aggregate the values themselves.
There was a problem hiding this comment.
Not sure where this comment stands in terms of implementation, but total power (and energy) sensors are very useful for the HA energy dashboard among other things. I've set up group and integral helpers, and given the prominence that energy has, I'd hazard a guess that I'm not the only one. While not hard for an experienced user to create, it's more thing to do multiplied by every user that wants it, and may be a little tricky for users new to Home Assistant. I'd gently suggest taking overall value into account.
There was a problem hiding this comment.
I think it makes more sense to implement aggregate sensors in the HA integration than here, that way it avoids any confusion between values reported by the device and ones generated from values reported by the device, since not all devices directly report things like total power out.
That distinction is perhaps a bit pedantic but SolixBLE is targeted at developers, where that might be important, wheras HaSolixBLE is targeted at users, who probably don't care as long as the sensor exists.
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>
The Prime Charging Station (A91B2) shares the Prime USB-charger per-port telemetry but is a base/CBC device, not Prime/GCM, so it subclasses SolixBLEDevice -- whose crypto is capability-driven (AES-CBC when the advert lacks the ECDH bit) -- rather than PrimeDevice, which would force GCM. The USB-charger decode lives on the class directly, without a shared PrimeUsbCharger base. It runs the full cleartext (0xxx) ECDH handshake in _initiate_negotiations and derives the CBC session key, then on connect confers the timezone and serial and starts the stream. Two telemetry frames are normalised onto one port view: the 4a00 snapshot (ports a4-a9, AC switches aa/ab) and the 4303 stream (ports a2-a7, remapped onto the snapshot tags). Tests lock in the snapshot decode and the stream remap. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add the Prime Charging Station 240w API page and its api.rst toctree entry, and give the A91B2 its own column in the Prime charger support table: the per-USB-port telemetry it shares with the 250w charger, plus the two AC-outlet switch states it decodes. Port and outlet control are not yet exposed, so those cells stay unchecked for now. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Expose on/off and auto-off timer control for the station's six switchable ports -- the two AC outlets and USB-C 1-4 -- following the 250w charger's grammar: per-port turn_ac_1_on / turn_usb_c1_on and set_timer_* methods, each calling _send_command with a shared ON_OFF / TIMER parameter template (4207 / 4209). The two USB-A ports report telemetry only, so they get no switch or timer. Port indices go in a2 (AC outlets 0/1, USB-C 2-5); ac_1, ac_2 and usb_c4 are confirmed from the app's cleartext BLE log, the rest inferred, as is the 4209 timer frame. Tests round-trip the on/off and timer frames through the CBC session key. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Because |
Delegate negotiation to SolixBLEDevice: the A91B2 advertises capability 00 (cleartext), so its handshake is the base Solix one. Delete the custom _initiate_negotiations/_exchange and override _process_negotiation only to capture the device serial (a4) from the 0829 stage, which the base parses but does not retain. Keep the 4303 stream alive via the native _keep_alive hook: the 420b realtime latch lapses ~8-10s after each trigger, so re-arm it on a 6s timer (as C2000G2 does with 4100) rather than leaning on a consumer's staleness poll, which otherwise re-ran the whole confer and churned the serial bind every cycle. Rename ac_1_switch/ac_2_switch to ac_output_1/ac_output_2 returning PortStatus, drop the aggregate usb_total_power_out (consumers can sum the per-port values), move the class docstring's technical detail into a collapsible note, and use hex-string packet patterns -- reusing const's NEGOTIATION_PATTERN, adding SESSION_PATTERN -- instead of bytes literals. Docs and tests updated to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Match the _data_summary naming the C2000 G2 review adopted for the base device's negotiation-derived state. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A2345 (PrimeCharger250w): - Merge the 4303 stream onto the ca00 snapshot (remap a2-a7 -> a4-a9) so ports read consistently and snapshot-only fields (sw_version, schedule/ timer, screen settings) survive the ~1/s stream; keepalive draws 4200 + 420b. - Add software_version, serial_number, per-port schedule/timer readback and set_schedule_*; fix set_timer to the 04<enable><u32 seconds LE> shape. A91B2 (PrimeChargingStation240w): - Add the display/charging-mode surface (0a00 ac/ad/ae/b4/b5): clock display + theme, screen brightness, screen timeout, charging mode, clock format, AC LED indicator -- readback + setters (4203-4206/4210/4214). The 4203 timeout setter enum differs from its ca00 field enum (see _SCREEN_TIMEOUT_CMD). - Move the keep-alive interval to a module constant. Shared: PortSchedule/PortTimer value classes with from_record; display enums. Tests cover the telemetry merge, readback decode, and setter frames. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
flip-dots
left a comment
There was a problem hiding this comment.
So I have taken a look at the docs changes and the C2000G2 changes and I don’t see any major issues with them but I am out of time to review this for this weekend so have not taken a look at the rest, though I plan on doing so next weekend.
My overall thoughts so far:
- Multiple PRs is better but still kind of difficult to review since comments and their fixes are spread across multiple PRs, I recently started using GitHubs stacked PRs for this kind of thing and that helps, you might want to consider using them or else keep PRs draft until all of its dependencies are merged.
- This needs to be split up more
- Support for the C2000G2 and devices which need button presses for pairing need to be a separate PR
- Support for dynamically determining the negotiation path needs to be its own PR
- Schedule support for the 250w charger needs to be its own PR
- Support for the 240w charger needs to be its own PR
- Support for using push and poll telemetry needs to be its own PR
- Be careful with method names/interfaces, they need to conform to the existing patterns.
| DC Power in status ✅ ✅ ❌ ❌ ✅ ✅ ❌ ✅ ❌ | ||
| DC Power out status ✅ ❌ ❌ ✅ ✅ ✅ ❌ ✅ ✅ | ||
| DC Timer ✅ ✅ ❌ ❌ ❌ ✅ ❌ ✅ ❌ | ||
| Max charge power ❔ ❔ ❔ ❔ ❔ ✅ ❔ ❔ ❔ |
There was a problem hiding this comment.
Stick to the convention of green tick and red cross here, while I do think the table could be improved by adding more symbols and I would be fine accepting a sensible PR which implements it, it should not be part of this PR.
| Polled status updates ✅ ❌ ✅ ✅ ❌ ✅ ❌ ✅ ❌ | ||
| ======================= ======= ========== ======= ======== ======== ======== =========== ===== ===== | ||
|
|
||
| The C2000 G2 (A1783) ``Pack voltage``, ``Cumulative energy out`` and ``Charge |
There was a problem hiding this comment.
Notes/warnings like this which are device specific should be in the device specific page, not the overall one.
| Serial number ❌ ✅ ❌ ❌ | ||
| ======================= ============= ============= ============= =================== | ||
|
|
||
| The 240w (A91B2) is an 8-in-1 charging station: it decodes the same per-USB-port |
There was a problem hiding this comment.
Notes/warnings like this which are device specific should be in the device specific page, not the overall one.
| @@ -0,0 +1,64 @@ | |||
| ================================= | |||
There was a problem hiding this comment.
I think there should be a page or pages in the docs about the different protocols but I don’t think this is the way to organise it.
There are multiple variants of the protocol, some use different forms of encryption, some use account binding, some are plain text, some need a button to be pressed, and some are a mix of encryption and plain text, and the earliest uses an entirely different packet structure.
I don't expect you to write all of that but maybe we structure it as a protocols page which has a section for each type, (i.e one for plain text, one for CBC one for GCM, one for solar bank) and at the top of the page we can have the details about which paths are used?
| self._authorized = True | ||
| elif plaintext[:1] == b"\x09": | ||
| _LOGGER.info( | ||
| "Device is awaiting a physical button press to authorize " |
There was a problem hiding this comment.
I think the library needs to respond in a more concrete way than just logging a message to tell you that you need to press the button, though what form that takes I am not sure, perhaps connect should raise a custom exception to tell the user they need to press the button and then they call connect again once they have? or maybe allow for a callback to be registered which is executed when a button press is needed? I think the second solution might be the neatest though I am open to suggestions.
| return bool(self._parse_int("a4", begin=23, end=24)) | ||
|
|
||
| @property | ||
| def display_on(self) -> bool: |
There was a problem hiding this comment.
The naming scheme is a tad inconsistent here but the existing convention for this particular thing is is_display_on, though display_on kind of makes more sense, I might end up changing the existing ones in a later PR but for now this needs to use the existing scheme.
| return bool(self._parse_int("a4", begin=22, end=23)) | ||
|
|
||
| @property | ||
| def display_brightness(self) -> int: |
| return DEFAULT_METADATA_STRING | ||
|
|
||
| # value_legacy retains the leading type byte. | ||
| block = self._data["f9"].value_legacy[1:] |
There was a problem hiding this comment.
You can use .value instead of .value_legacy here.
| return self._version_slot(0) | ||
|
|
||
| @property | ||
| def software_version_sub_mcu(self) -> str: |
There was a problem hiding this comment.
Does it make more sense to use software_version_controller here?
| return fields if len(fields) >= SUB_PACKAGE_TAIL_LENGTH else None | ||
|
|
||
| @property | ||
| def expansion_present(self) -> bool: |
There was a problem hiding this comment.
I think it makes more sense to move this into num_expansion, since num_expansion is used by other devices.
The problem with stacks is that it also needs push permission, so when I tried, I got: |
Split out of #45 per review — live BLE telemetry for the Anker Prime chargers.
Adds the Prime USB-charger base (
prime_usb_charger) carrying the shared4303/ca00telemetry decode, and builds the A2345 (250W charger) and A91B2 (240W charging station) devices on it: per-port voltage/current/power/status, total output, per-port switches, and the station's two-frame-layout / AC-switch specifics.Stacked on #50 (c490) → #49 (negotiation) → #48 (reassembly). GitHub shows the cumulative diff until those merge; this PR's own contribution is the last commit (the Prime device support). 123 tests pass.