Solarbank 2 Schedule/Power control - #36
Conversation
- Exposes SolixBLE set_schedule(power_w) flip-dots/SolixBLE#36 as a user-facing control. - Number entity (0-800 W, 10 W step, slider) stages the target value; - Apply button to commit it to the device. - Last value restored across HA restarts via RestoreEntity. - Library errors surface as HomeAssistantError so they show as UI notifications. - No Version increment yet -> Maintainer - Add Solarbank 2 to README supported devices list - Added unit tests
| async def _initiate_negotiations(self) -> None: | ||
| """Start the legacy base SolixBLEDevice handshake.""" | ||
| _LOGGER.info("SB2: starting legacy base handshake (00xx/08xx, AES-CBC)") | ||
| await super()._initiate_negotiations() |
There was a problem hiding this comment.
I think it makes sense to cut this since _initiate_negotiations() already produces a log entry when called.
There was a problem hiding this comment.
This log output is meant to let the reader now whether legacy or prime style handshake was used for the connection. _initiate_negotiations() does not provide this information. Is it ok to leave it in there for now?
There was a problem hiding this comment.
Sure, but maybe don't call the old style "legacy" just yet given that the new one is very experimental. I think its only fair to call it legacy when the newer style is able to fully supersede it (including being able to generate these IDs).
|
|
||
| # Encryption helpers | ||
|
|
||
| def _encrypt_with_static_key(self, plaintext: bytes) -> bytes: |
There was a problem hiding this comment.
When I originally implemented the prime protocol I think I just stored the cipher text in constants instead of storing the plain text and then encrypting it before sending it since they were constants, but since you now need that functionality I think it would make more sense to implement it in the _encrypt_payload() function of prime_device.py.
In _decrypt_payload() it automatically determines if it should use the static or dynamic key depending upon if the dynamic key is set, you should be able to use that to implement something similar for _encrypt_payload()
There was a problem hiding this comment.
Does this still hold true with all the new developments in the other devices? If yes, I'll move it. Please let me know.
There was a problem hiding this comment.
It holds true for prime devices, the older style Solix devices do not use encryption with a static key for the first few messages, those are all in plain text.
| .. note:: | ||
| A pristine, never-paired SB2 may still require a one-time pairing | ||
| through the Anker app before any BLE client (including SolixBLE) | ||
| can connect. |
There was a problem hiding this comment.
Apologies if this is already explained elsewhere and I just missed it, but how would one go about getting this cloud ID, does the app show it anywhere?, if you use the legacy protocol is it included in the telemetry data? or might there be some command in the legacy protocol to request it that could be found in the dumps?
There was a problem hiding this comment.
So the ideal workflow for the future is:
- We reverse engineer the pairing process (not the handshake, the actual first time pairing with the button press).
- I am 99% sure this id is transferred during pairing
- We implement our own pairing in SolixBLE
- User can set whatever ID he/she pleases
In the meantime maybe it's possible to get the ID from the cloud API, but I haven't tried because I knew mine from the captures
|
|
||
| Uses 40xx/48xx negotiation across 8 stages and AES-GCM for session | ||
| traffic. Requires an Anker user-id. SB2 firmware whitelists user-ids | ||
| and rejects unknown values with RX 4827 = ``09 a1 02 b4 00``. |
There was a problem hiding this comment.
It wouldn’t happen to accept a 0000000 user ID would it? Its probably wishful thinking but we have seen Anker take a few shortcuts when it comes to security so I wouldn’t be completely surprised.
There was a problem hiding this comment.
No it rejects a random user ID. But as I said above, reverse engineering the pairing process is probably very easy now. I simply ran out of time
There was a problem hiding this comment.
Fair enough. Given that this needs someone to go through the effort of finding the ID I think this needs to remain experimental for now. I am fine with merging it but it needs to be clearly labelled in the docs as experimental.
flip-dots
left a comment
flip-dots
left a comment
There was a problem hiding this comment.
You know I gotta hand it to you, this is some great work, its highlighted quite a few shortcomings with the existing abstractions I made which I should probably fix at some point, hell most of my review comments are just me highlighting things for future me to do.
In theory my availability should be a lot better now that my university exams are sorted so feel free to reach out if you need additional clarification or some help with this.
|
Thank you so much for taking your time to review this big blob so thoroughly. I appreciate it a lot! I will consider every one of your comments, I just replied to the obvious ones where I don't need to change much on my side. With that being said my current availability is very poor for the next few months, so I don't know when I'll find the time to work on this again. If someone else wants to pick this up in the meantime I'm fine with it and I'm happy to help. |
|
Noticed a bug today in "production": Device reports input_cutoff_data = 100 % Possible causes:
Either way this crashes the HA integration. Needs fix before merge @jul1an-s . Error message in HA |
|
The Input cutoff field is now used as max soc, but only if the device firmware supports it. |
|
Hi @thomluther, thank you for sharing. Could you elaborate a little further? The behavior of the field is not clear to me:
|
This is the old SOC reserve command structure (4067 or MQTT 0067): Those 3 fields had only 2 options each, and their state was in 0405 messages: I'm not sure what that field was used for, it was inherited from Solarbank 1. Maybe it was always ignored. The new SOC limit command structure uses different fields except a2 (4067 or MQTT 0067): The output_cutoff_data b4 field was always the SOC reserve, and is now also used as new min SOC value. The b6 field is presenting the used charge SOC limit, that is typically identical with the max SOC. The eb field is new and shows the max SOC setting from the command: It could also be that the original field always had the same meaning 'input_cutoff_data' => limit for the charge, but maybe this is ignored by the firmware, if not larger than min soc, or 'output_cutoff_data', this the battery was always charged to 100%, since the old SOC reserve command always set this field the same value as the minimum... Anyway, you need to find a way to recognize from the installed firmware level or 'eb' field value, whether the old or the new SOC command structure should be used. The old command should not be used anymore with the newer firmware, since it may invalidate the max soc setting. I should also note that Solarbank 2 does not support the backup soc or switch settings to my knowledge, but the App always sends all 4 fields, where the 2 backup fields are always 0. Not sure if they are really needed in the command structure. |
|
@flip-dots I think we're ready to merge. No need to prolong this any longer. I will tackle the input cutoff problem later when I have time and I'm planning to rework the set power to make it clear we are writing a schedule there. |
flip-dots
left a comment
flip-dots
left a comment
There was a problem hiding this comment.
There are a few things that I would like to see addressed, most importantly the statically defined time zone, though there is some code which might help you there, that and the bits of code from the solarbank1 that might be compatible which could simplify things a lot.
Its worth noting that there are a few things in progress at the moment which solve a few of the problems you have had to solve yourself (timestamps, time-zones, negotiation) and you might want to wait for those to get merged before continuing on (though I don't mind if you don't want to wait).
| async def _initiate_negotiations(self) -> None: | ||
| """Start the legacy base SolixBLEDevice handshake.""" | ||
| _LOGGER.info("SB2: starting legacy base handshake (00xx/08xx, AES-CBC)") | ||
| await super()._initiate_negotiations() |
There was a problem hiding this comment.
Sure, but maybe don't call the old style "legacy" just yet given that the new one is very experimental. I think its only fair to call it legacy when the newer style is able to fully supersede it (including being able to generate these IDs).
|
|
||
| Uses 40xx/48xx negotiation across 8 stages and AES-GCM for session | ||
| traffic. Requires an Anker user-id. SB2 firmware whitelists user-ids | ||
| and rejects unknown values with RX 4827 = ``09 a1 02 b4 00``. |
There was a problem hiding this comment.
Fair enough. Given that this needs someone to go through the effort of finding the ID I think this needs to remain experimental for now. I am fine with merging it but it needs to be clearly labelled in the docs as experimental.
|
|
||
| # Encryption helpers | ||
|
|
||
| def _encrypt_with_static_key(self, plaintext: bytes) -> bytes: |
There was a problem hiding this comment.
It holds true for prime devices, the older style Solix devices do not use encryption with a static key for the first few messages, those are all in plain text.
| # The tz_offset is signed LE seconds; CEST in capture was -7200 | ||
| # (= -2h). We hardcode that for now - proper localtime detection | ||
| # is a TODO. | ||
| tz_str = b"CET-1CEST,M3.5.0,M10.5.0/3" |
There was a problem hiding this comment.
I think this is too important to not implement. #45 changes how negotiations work so the time is correct for other devices where that matters (e.g chargers/power banks with the built in clock) and there is some code in that for generating time zones that might be useful.
You could also wait for that code (or something similar which fixes times) to be merged since its a relatively high priority and I will probably end up addressing it soon.
| # is a TODO. | ||
| tz_str = b"CET-1CEST,M3.5.0,M10.5.0/3" | ||
| tz_offset = (-7200).to_bytes(4, "little", signed=True) | ||
| pt = ( |
There was a problem hiding this comment.
Does the timestamp need to be at the beginning of the packet rather than the end?, usually its at the end. If it works with it at the end you can probably use _send_command() to have it auto-generated instead of needing to generate it for all of these.
| """ | ||
| return bytes.fromhex(f"a10121a2020100a30201{0 if light_on else 1:02x}") | ||
|
|
||
| async def set_light_switch(self, light_on: bool) -> None: |
There was a problem hiding this comment.
For these kinds of things the rest of the library uses dedicated turn_off and turn_on functions (e.g here) it would be best to stick to that convention.
There was a problem hiding this comment.
That and it makes it easier to add to the Home Assistant integration since it expects switches to work that way.
| ) | ||
|
|
||
| # Mapping for 3rd field reserved power command | ||
| _RESERVED_POWER_A3_MAP: dict[int, int] = {5: 4, 10: 5} |
There was a problem hiding this comment.
Why put it here instead of at the top of the file with the rest of the constants?
| # 0x405e set-schedule | ||
|
|
||
| @staticmethod | ||
| def _build_set_schedule_payload(power_w: int) -> bytes: |
There was a problem hiding this comment.
As per this, if you are able to use the FamilyLoadSchedule from #27 that would be really good. If not then it would be good for this code to use the same dataclass structure as is used there. I feel like this is just a tad too complicated to put in the device class and it would also be easier to test and document if it was separate.
| # Decrypt for logging; _decrypt_payload picks static vs session key | ||
| # automatically based on whether _shared_secret has been set. | ||
| try: | ||
| decrypted = self._decrypt_payload(payload) |
There was a problem hiding this comment.
Is this needed? _decrypt_payload() should automatically determine when to use the static/session key based on if the session key variable is None or not.
|
Yeah I am aware that there are a lot of things going on in this project right now which make this lib pretty much a moving target. Honestly I'm not sure if my availability for this project allows me to get this merged given that some people appear to be working at full auto-vibe code speed. I can try to work on your latest comments soon to get this merged before the great refactor. But if I need to rework and retest the entire inner machinery I guess I'll just leave it to someone else with better availability. Please let me know how you want to proceed |
|
I 100% get it, its really easy to get drawn into something and then find out its a massive hassle with a seemingly infinite number of change requests and a moving target, I experienced the exact same thing when I added support for Bluetooth Hue lights to Home Assistant, first it was the tests, then its the layout/structure, then they deprecated an API I used, and then they added all these requirements for 3rd party libraries to be built with CI/CD and be auditable and it was a total nightmare, I think it ended up taking over a year for all of it to go through and even now there are still issues with it. There is a good reason for it though, as the main developer of this thing I am taking on responsibility for maintaining it which means I need to be able to understand and maintain it, as well as anyone else who might need to change it and I don't have the hardware to test, so making unilateral changes later to things like packet decoding and negotiation is quite risky, tests can help mitigate most of it but not all. In terms of the more practical aspects I am hoping some of these soon to be implemented changes (like timestamps and timezones being handled by the base classes) will be able to significantly shrink this PR which shrinks the amount of code that I have to maintain that I am not able to test on hardware. I don't expect the big PRs that change a lot of the things to be merged soon, there are many issues with them as it stands, I suspect I will end up making my own which implement just the minimum. In terms of timescales I am happy to leave this PR as draft for as long as needed and I might end up building something to implement timestamps and timezones for all devices in the next few days since I now have a device where the time actually matters (one of those desktop chargers which has a clock). If your availability does not allow for you to sort all of this out yourself I don't mind implementing the improvements myself and I could probably do that some time in the next month or two after the timestamp/time-zone changes are done, so long as you are willing to test it and make sure it works and send me any test data I might need. |
|
I totally understand that you as a maintainer have a great interest in keeping the codebase lean and maintainable. I'm sorry that you had so much hassle with your HA project :-( However I can understand them having thousands of users and hundreds of contributors. A project of that size needs this kind of thoroughness (especially given that they are doing very critical things like door locks etc). Nevertheless you should consider two aspects:
Regarding the size of this PR we could simply drop the frida_dart script which is 1/3 of the PR and the "SolarbankPrime" path which is probably another 1/3 and not wired to your HA repo anyways. I checked it in to document my findings for future contributors, but ofc it introduces maintenance overhead for you. It can also rest on my harddrive like other stuff I discovered which is not part of this PR (OTA flashing etc) Sure we can leave this open if you have time to work on it in the close future, but I really wanted to ship this to "normal" users in HA soon given some comments in the HA repo which suggest this is already being used by some people 🤔 Especially with Anker having officially denied all hopes for an official solution for this device. |
|
Hi Julius, we have now integrated an owned Solarbank 2 E1600 AC (A17C0) locally via BLE. Your dynamic/Prime-style handshake work was very helpful, and a few observations from our captures may be useful for your PR:
This may be specific to the A17C0 / Solarbank 2 AC dynamic path, so we would keep it guarded and separate from the legacy SB2 handshake. Best regards, |
5 participants
Changes:
405e. Currently restricted to setting one power output 24/7[untested]:Tests & Docu
Why two handshakes?
As I mentioned in the issue #28 I have implemented both handshakes/protocol variants:
Prime handshake
I included the Prime style variant because it seems like the "right" way to interact with the device and a lot of work went into it. The legacy handshake might get removed any time, so it's good to have a fallback option.
"Legacy" handshake
I included the Legacy variant because right now it is so much simpler for our use case. No env vars, no user specific config.