Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wallbox Integration #153

Merged
merged 15 commits into from
Jun 30, 2024
Merged

Wallbox Integration #153

merged 15 commits into from
Jun 30, 2024

Conversation

bullitt186
Copy link
Contributor

As discussed in #47, i implemented the Wallbox features of python-e3dc 0.9.2.

I could test the features on my S10X compact with a MultiConnect II:

  • Sensor readings work and are plausible
  • Sun Charging can be set
  • Phases can be toggled
  • Max charging current can be set via service

I wasn't able to validate

  • Schuko (as the Multiconnect II does not have one)
  • Toggle Charging (as my car was fully charged today)

As of now, only 1 Wallbox is supported.
As more than one Wallbox requires some entity name juggeling, i skipped this for now.
I had to add the button entity to this integration for the toggles.

I am happy for testers, suggestions and improvements.

@torbennehmer
Copy link
Owner

Hi @bullitt186,
thank you very much for this PR, most of it is great work, I added a lot of minor things to keep the codebase in the same style, along with a few other minor questions about changes in the sourcebase. Apart from that, I'm very happy with the general approach you have in here, great work!
Cheers, Torben

@bullitt186
Copy link
Contributor Author

Hi Torben, Thanks for the positive feedback 👍
I am curious to see the changes you did but find only your comment here but no other commits or change request to the pull request. Am i missing something, looking in the wrong places or did you integrate it locally on your end and will push later?

Please let me know if you would like to see any other changes from my end before integrating the PR.

by the way in the meantime i was able to verify that the toggle for "Wallbox Charging" works as expected.

Cheer,
Bastian

@torbennehmer
Copy link
Owner

Ah Bastian, I'm very sorry, that got lost in translation.
I did not change anything, I added comments to the PR by using a PR review, you should see them in the changes view of the PR.
Cheers,
Torben

@bullitt186
Copy link
Contributor Author

Hmmm, That's odd. But maybe it's also a layer 8 problem on my end.
I cannot find your remarks anywhere.
I would have expected to see such a "changes requested" mark as shown in the screenshot or as you said in the changes view. In my last PR it also was like that.
https://docs.github.com/assets/cb-157392/mw-1440/images/help/pull_requests/merge_box/pr-reviews-in-merge-box.webp

Could you please doublechecked whether you submitted your PR review?

ps - while searching for your comments, i realized my local PyLinter changed more formatting as I intended and this must have missed my attention. sorry. will fix this if neccesary.

.DS_Store Outdated Show resolved Hide resolved
.devcontainer.json Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
.vscode/settings.json Show resolved Hide resolved
custom_components/e3dc_rscp/binary_sensor.py Show resolved Hide resolved
custom_components/e3dc_rscp/strings.json Outdated Show resolved Hide resolved
custom_components/e3dc_rscp/switch.py Show resolved Hide resolved
custom_components/e3dc_rscp/switch.py Show resolved Hide resolved
custom_components/e3dc_rscp/switch.py Outdated Show resolved Hide resolved
custom_components/e3dc_rscp/translations/en.json Outdated Show resolved Hide resolved
@torbennehmer
Copy link
Owner

Hmmm, That's odd. But maybe it's also a layer 8 problem on my end. I cannot find your remarks anywhere. [...]
Could you please doublechecked whether you submitted your PR review?

Nah, it was my Layer 8 :-) Klicking on "Submit review" helped, I guess. Sorry for that.

ps - while searching for your comments, i realized my local PyLinter changed more formatting as I intended and this must have missed my attention. sorry. will fix this if neccesary.

Yeah, thought that. If you have an idea how to configure the VSCode Workspace to have this consistently across all dev systems, I'd be happy to take an independant ticket and PR for this. I don't stick to a specific style of formatting, it just should be consistent, so that we don't reformat back and forth between us :) This probably has to include line lenghts, linter and formatter types etc., which would have to be set in the workspace config, not in the user config. I'm no expert for the options we've for vscode Python development, to be honest, so I'd be happy for input on this end.

Cheers, Torben

@bullitt186
Copy link
Contributor Author

Ah yes, now i have your feedback to work with :-) Thank you.
Let's see whether i find the time tonight to get it done.

Regarding linting: I used my lunch break to explore this a little bit. I put my thoughts/results in this issue: #154
Let's continue the discussion on linting there.

@neuromancer3142
Copy link

Hi guys,

Any way I can support here wrt. end user testing? I do not see the new E3DC version in HACS for testing yet. Please let me know when I can test from my end. I do have an S10E Pro with an E3DC Wallbox Easy Connect.

Cheers.

@bullitt186
Copy link
Contributor Author

Hi, I just committed the changes / review remarks by Torben after some more testing on my end.
There's one thing i don't get...
In the clip below, you see the switches fall back to "unknown" but i can't figure out why.
The switches appear identical to the others of the extension (e.g. config switch weather regulation) which acts differently.
The switch is not updated by me somewhere else.
Just to be clear: the state of the wallbox (e.g. sun mode) is changed correctly and does not fall back, but HA "forgets" the switch state.
@torbennehmer - do you have any ideas what may cause this behaviour?

@neuromancer3142 - in order to find these changes in HACS, they need to be fully integrated + released first.
if you want to test them in it's current state already, you'd have to clone my fork (https://github.com/bullitt186/hacs-e3dc/tree/wallbox) or download it's zip and place the content of custom_components in your config folder of HA.
(or open the devcontainer in vscode and then run /scripts/develop which spins up a local instance of Home Assistant)

I'd be curious to get some feedback.

Cheers!

Bildschirmaufnahme.2024-06-27.um.23.05.00.mov

@neuromancer3142
Copy link

@neuromancer3142 - in order to find these changes in HACS, they need to be fully integrated + released first. if you want to test them in it's current state already, you'd have to clone my fork (https://github.com/bullitt186/hacs-e3dc/tree/wallbox) or download it's zip and place the content of custom_components in your config folder of HA. (or open the devcontainer in vscode and then run /scripts/develop which spins up a local instance of Home Assistant)

I'd be curious to get some feedback.

Happy to provide feedback. Easiest way for me would be to have it in one of the next releases and then give it a go. If that is not happening anytime soon, I will look into the download part. I fear of breaking my HA instance and not being able to fix it ;-).

Cheers.

@torbennehmer
Copy link
Owner

Happy to provide feedback. Easiest way for me would be to have it in one of the next releases and then give it a go. If that is not happening anytime soon, I will look into the download part. I fear of breaking my HA instance and not being able to fix it ;-).

We can push out a Beta release, I'll look into the PR today and check that.

@neuromancer3142
Copy link

Thanks a lot Torben ...

@bullitt186
Copy link
Contributor Author

I think i found the cause for the odd switch behaviour, am currently working on it. so you may want to hold back your efforts for the beta release for a little bit.

@torbennehmer
Copy link
Owner

Hi @bullitt186,

[...] There's one thing i don't get... In the clip below, you see the switches fall back to "unknown" but i can't figure out why. The switches appear identical to the others of the extension (e.g. config switch weather regulation) which acts differently. The switch is not updated by me somewhere else. Just to be clear: the state of the wallbox (e.g. sun mode) is changed correctly and does not fall back, but HA "forgets" the switch state. @torbennehmer - do you have any ideas what may cause this behaviour?

I have seen this behavior before, yes. What I saw with weather regulated charging was about this:

  1. User clicks the switch, it flips
  2. Code behind this pings E3DC to update the setting
  3. Meanwhile, E3DC updates the entity based on the coordinator's state, which still has the old(!) value of the setting. This is because you're racing against the next E3DC poll.
  4. At the next poll (10s later max.) the integration polls E3DC again and then updates to the new setting.

I had to work around this in the coordinator to actually update the state immediately when I'm sending service commands to E3DC. Basically, you have to optimistically update the integrations internal state, hoping that the next poll will actually match this. Look at the following line for an example.

self._mydata["pset-weatherregulationenabled"] = enabled

Another example:

self._mydata["pset-powersaving-enabled"] = enabled

You'd still have a last edge case in case a poll is already going on while you're temporarily patching the coordinator state. I haven't seen that so far, but in theory that's possible as we're running multi-threaded.

That's why I'm using a guard variable to protect against this. I'm not sure if this is a true Python synchronisation style, but it did work so far. Any insights from a Python expert in multi-threading would be appreciated here :)

self._update_guard_powersettings = True

Cheers, Torben

@bullitt186
Copy link
Contributor Author

That's why I'm using a guard variable to protect against this. I'm not sure if this is a true Python synchronisation style, but it did work so far. Any insights from a Python expert in multi-threading would be appreciated here :)

I guess as well there's a more native way to handle mutex but for now I copied your style.

With the latest commits i did some more refinement, in summary:

  • added mutex behaviour
  • made switch and state to be in sync (with the limitations stated by @torbennehmer above)
  • changed the binay_sensors to native HA classes, this made it necessary to rename the entity names
  • realized i had some strings/translations at the wrong entity type and corrected this.
  • set the entities which are specify to WB type multi connect I (schuko) and easy connect (plug locked state) to disabled by default in order not to confuse users. I haven't found a way to identify the WB type, ideally the user could select their WB type during set up of the integration
  • more testing next to the car to verify the states / behaviour

In accordance to the quote "don't let perfect be in the way of better" i think this is ready to release to early adopters.

@torbennehmer
Copy link
Owner

cool thing, I'll go over it right away.

@torbennehmer torbennehmer merged commit 13a9219 into torbennehmer:main Jun 30, 2024
5 checks passed
@Thomansky
Copy link

I have the problem that only the settings for a wallbox are set for me and the battery score does not work for me from the car either, it is permanently displayed at 100%.

@Thomansky
Copy link

All settings, i.e. phase switching, sun mode and other settings, are only occupied for one wallbox in my case, so I can't say whether the car battery score works because I haven't yet tried connecting it to the other wallbox where all functions work, so it may be that it does work, but only for the one wallbox I have two E3/DC Wallbox easy connect

@bullitt186
Copy link
Contributor Author

Hi @Thomansky
Thank you very much for your initial feedback.
In fact, as of now all functions only work with the primary wall box or to be more precise for the Wallbox with the lowest ID.

I would expect that it is consistent throughout all sensors and switches that only one of the wall boxes information are shown or settings can be set. There should not be a mix between different wallboxes.

Supporting multiple wall boxes is on the to do list but probably won't make it into the initial release.

Regarding state of charge: I guess this depends on the communication between the specific vehicle and the wall box.
In my case I have a Mercedes EQB and a multi connect 2 wall box and state of charge is always zero.

I will investigate this a little bit.

@Thomansky
Copy link

Thomansky commented Jun 30, 2024

Okay, I've just connected my car. Battery level 96% in the Home Assistant is currently displayed at 3% but every 30 seconds it increases by 1%, I'll wait until it reaches 96% maybe it's just an update problem.image

@Thomansky
Copy link

The car's charge level actually seems to be updated by 1% every 30 seconds until the value in Home Assistant is at the car's current value

@Thomansky
Copy link

Is it not possible to choose which wallbox is prioritised?

@bullitt186 bullitt186 deleted the wallbox branch June 30, 2024 14:57
@bullitt186
Copy link
Contributor Author

@Thomansky Within the HA integration, i just pass the values received by the Hauskraftwerk via a python library (https://github.com/fsantini/python-e3dc) through and am not altering any values. Maybe this is the system behaviour of the hauskraftwerk learning the SoC from the vehicle?
Did you plug in the car just before checking the values?

Wallbox prioritization is not implemented in this library and i haven't seen this feature in E3DCs RSCP spec, so I'm afraid this is not available. However maybe you can mimic prioritization within HA with automations in a creative way?

I created a new PR #155 which supports multiple wallboxes, so you may be able to use/test multiple ones soon.

@Thomansky
Copy link

Thomansky commented Jun 30, 2024

I will check it again tomorrow with the score. Unfortunately, my car is full, and the value has not been updated for 5 hours. I suspect that the 30 seconds come from the update rate that the integration sends to the house power plant. In the app Autarkie Manager, there is an option to set different update rates for power data and system parameters. Currently, I have an update rate of 10 seconds for power data. It might be a useful feature to add to the integration, allowing users to decide how often it updates. I apologize for my poor English; I am using a translator!image

@torbennehmer torbennehmer added the enhancement New feature or request label Jul 2, 2024
@T1ppes
Copy link

T1ppes commented Jul 15, 2024

I finally managed to update to the latest beta. Generally working and happy with the added features. I cannot provide any further feedback to the already mentioned. Although I currently have only one wallbox, I’d support to have the wallbox as a different device compared to the converter.
State of Charge provided by the wallbox does also not always align to the actual SoC of my car.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants