search

twhittock / eo_mini

EO Mini integration for Home Assistant
MIT License
13 stars 6 forks source link

Charging options #4

Closed badguy99 closed 1 year ago

badguy99 commented 1 year ago

Adds charge options to turn on/off Off-Peak / Solar / Scheduled Charging. Also adds a number entity for the minimum current for solar charging.

These seem to work ok with my setup. I could turn the charge modes off/on and change the minimum charge limit and that was reflected in the EO Smart Home App.

There is probably one caveat around having a time schedule defined for the things being turned on already via the app. Ideally I would have liked to have added entities to cover the start/stop times for the three different charge types. But I think it would be best to do this with a time entity and looking at the PR that added that, it looks like that may be coming later in 2023.6

twhittock commented 1 year ago

Looking forward to the changes you mentioned. Note the tests failed in that version too. Probably some fragile dependency on the interface copied into the tests.

twhittock commented 1 year ago

So I think the charge options dictionary should be updated by the response from the server, not our code, so we know that the value was validated and actioned by EO, not a hangover of making a request (which might have been rejected or something by the server.) Also the tests are failing, but I'm keen on merging this soon, sorry for pushing back on your contribution - I hope we can merge soon!

badguy99 commented 1 year ago

So I think the charge options dictionary should be updated by the response from the server, not our code, so we know that the value was validated and actioned by EO, not a hangover of making a request (which might have been rejected or something by the server.)

I think you are going to need to give me some code examples/snippets of what you want here as I don't understand what you are trying to say I'm afraid. I can then try them with my changes and see how I get on, but we do need to send the whole dictionary I think, as from my testing when I didn't things got disabled.

Also the tests are failing,

I don't believe this is down to my changes, or at least I can't see anything that would cause the issue - it seems like the issue raised here - https://github.com/MatthewFlamm/pytest-homeassistant-custom-component/issues/158 - which it looks like you commented on already

but I'm keen on merging this soon, sorry for pushing back on your contribution - I hope we can merge soon!

No worries :) I'm not going to be able to do more on this for a couple of weeks, but will pick it up when I'm back again

badguy99 commented 1 year ago

I'm going to close this. I think it needs further work and the time entities to work together so that they are all set and only pushed at once to the EO API, when another "write" switch is set. Unexpected things happen if the different modes are turned on without times being set, and the times seem to be unset when the modes are turned off. I don't have the time to work on this any more though