S10 fixes

[torbennehmer]

A number of fixes done for my HA integration to work cleanly.

fixes #73:

• S10E Serial number detection did not work, a missing elif masked the corresponding check
• A few exported classes were missing (notably SendError for error detection)
• Calls to set_power_save and set_weather_regulated_charge were not correctly parsing the results from E3DC. (Did some code streamlining to those functions on the way)

[vchrisb]

Thank you for your contribution.
I made a few comments.

[vchrisb]

Using rscpFindTagIndex is nice.

EMS_RES_WEATHER_REGULATED_CHARGE_ENABLED is not returning the actual set value but rather 0 if the new value was set successfully.
Therefore checking against 0 was correct.

[torbennehmer]

Maybe the S10 is behaving differently here? I see things differently here, basically:

A call e3dc.set_weather_regulated_charge(False) to my S10 always yields this result, with weather regulated charging successfully disabled:

[
  "EMS_SET_POWER_SETTINGS",
  "Container",
  [
    [
      "EMS_RES_WEATHER_REGULATED_CHARGE_ENABLED",
      "Char8",
      0
    ]
  ]
]

This is a dump of the res structure directly after teh call to sendRequest. If I call e3dc.set_weather_regulated_charge(True), I always get this:

[
  "EMS_SET_POWER_SETTINGS",
  "Container",
  [
    [
      "EMS_RES_WEATHER_REGULATED_CHARGE_ENABLED",
      "Char8",
      1
    ]
  ]
]

Again, independant of the number of calls, the feature is enabled successfully. This is what I see reverse-engineering my own unit. What I was unable to do though is trying to force an error. So I can't tell how this call behaves if for any reason the S10 could not set the value.

How does your unit behave?

[torbennehmer]

@vchrisb should I drop the Weather Regulated Charge changes from this PR until these questions are resolved so that the other changes can be merged?

[vchrisb]

I'll have a look in the evening at my E3DC how it behaves.

[vchrisb]

1 is enabling weather regulated charge any other value is disabling it.
The return value is 1 for True/1 and 0 for all other positive integers.
E3DC API consistency...

[vchrisb]

Can you test again?
You shouldn't be able to send 3000 at all, as it is larger than 255.
Further up you did report the same behavior like me. I do not expect E3DC to implement APIs differently.

set_weather_regulated_charge should only need to support boolean. We might add type annotations.

[torbennehmer]

I have created a test run, the code it uses is attached to this file. It basically calls set_weather_regulated_charge repeatedly and rereads the current state afterwards alongside the return value of the call itself. What I get is this:

WRC state: False
Setting WRC to int 0, got 0, new state: False
Setting WRC to int 1, got -1, new state: True
Setting WRC to int 0, got 0, new state: False
Setting WRC to int 2, got -1, new state: True
Setting WRC to int 0, got 0, new state: False
Setting WRC to int 5, got -1, new state: True
Setting WRC to int 0, got 0, new state: False
Setting WRC to int 3000, got -1, new state: True
Setting WRC to int 0, got 0, new state: False
Setting WRC to int 0, got 0, new state: False
WRC state: False

sample.txt

Explicitly, there seems to be no error when sending 3000 to the API (i understand, that it's an 8 bit type, I'd have expected an exception here, but did not dive further in the code. Maybe python casts this implicitly, but honestly, its probably beside the point. If we define the argument as bool, this will be of no more importance. The Int 3000 correctly runs on an error, the current master code does check this beforehand, so that the 3000 never reaches the E3DC RSCP code. I beg your pardon here, I had another piece of code in my mind.

Essentially, if we break it down to zero/nonzero, we are identical. What I think is, that there are subtle details in the hardware, one returning 0x01 for active, the other 0xFF. I'll rebase this branch onto the current master and build you a new proposal.

[torbennehmer]

The new version of this branch should now fix this. current test output:

WRC state: False
Setting WRC to int 0, got 0, new state: False
Setting WRC to int 1, got 0, new state: True
Setting WRC to int 0, got 0, new state: False
Setting WRC to int 2, got 0, new state: True
Setting WRC to int 0, got 0, new state: False
Setting WRC to int 5, got 0, new state: True
Setting WRC to int 0, got 0, new state: False
Setting WRC to int 3000, got 0, new state: True
Setting WRC to int 0, got 0, new state: False
Setting WRC to int 0, got 0, new state: False
WRC state: False

This should now correctly give us the result state. Unfortunately, I could not bring the E3DC to actually return an error (no idea how I can get the call to fail), but the happy path does work.

[vchrisb]

I can't remember that we hand an inconsistency in the past. AFAIK E3DC does ship the same firmware for the different S10 models.
Which model and release version do you have?
It would be great to find other folks to comment on their observations before proceeding.

FWIW the E3DC support was helpful in the past to shed some light on these topics.

[torbennehmer]

Contacting support is a good idea (even though I've got one open ticket for 2-3 weeks now w/o response, but not trying will be even less helpful). Unfortunately, in the RSCP-Tags-Official.xlsm there is no clear documentation on the return value. It sure sounds that it schould return the current value.

I've got an S10E from 2015, Firmware Version S10_2023_02, Serial Number starts with 4515... What do you use?

[torbennehmer]

@vchrisb I have just created another PR without the changes around weather regulated charging. That should allow you to move on, while we get to the bottom of this here :)

#75 open

[torbennehmer]

@vchrisb How should we continue here, I still think we have some different behviour here depending on unit type. If this is easier for you, we could also take this discussion offline and do it in German language.

[eikowagenknecht]

If you need to test with yet another device (S10 X Compact 14), feel free to contact me :-)

[waldbaer]

I can confirm that I have the same issue with the set_weather_regulated_charge() return value when enabling the feature.
Used E3/DC product: S10
Firmware version: S10_2024_02.