torbennehmer / hacs-e3dc

Homeassistant E3DC Integration
GNU Affero General Public License v3.0
53 stars 8 forks source link

Wallbox Integration #153

Open bullitt186 opened 5 days ago

bullitt186 commented 5 days ago

As discussed in https://github.com/torbennehmer/hacs-e3dc/issues/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:

I wasn't able to validate

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 commented 2 days ago

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 commented 2 days ago

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 commented 2 days ago

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 commented 2 days ago

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.

torbennehmer commented 2 days ago

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 commented 2 days ago

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: https://github.com/torbennehmer/hacs-e3dc/issues/154 Let's continue the discussion on linting there.

neuromancer3142 commented 1 day ago

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 commented 1 day ago

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!

https://github.com/torbennehmer/hacs-e3dc/assets/24450990/119a5a40-03ad-4a5d-a337-30daa8cb14de