cliviu74 / wallbox

Python Module interface for Wallbox EV chargers api
Apache License 2.0
62 stars 22 forks source link

Adding a global timeout support, and setting a default timeout (+ retry) + powerboost + schedules #34

Closed tmenguy closed 6 months ago

tmenguy commented 6 months ago

Hi,

The today implementation has used in Homessitant is used without applying a timeout to the wallbox library .... so the library hangs once in a while due to the Wallbox server not answering. Anyway from the doc, request shouldn't be used without a timeout set. I've added a default one, a number of retry in case of timeout, and a timeout not only on get request but on all request in the library.

UPDATE : added the support for schedule and ICP from https://github.com/cliviu74/wallbox/pull/35 (thanks https://github.com/nanomad !)

Hope this help! BR Thomas

tmenguy commented 6 months ago

Do not hesitate @cliviu74 if you have any questions here, I implemented this fix has my eallbox installation was stalling randomly and I had to relaunch it a lot…. And then I see that by default in HA component, the timeout was left at None (so infinite).

nanomad commented 6 months ago

@tmenguy do you have a way of testing your branch with HA?

cliviu74 commented 6 months ago

Hi @tmenguy please split changes into individual pull request, changes for ICP and PowerBoost implementation need to be split from the timeout changes./ Also can you please let me know when changes to are ready to PR. I see there are commits still added to the PR which makes it difficult to merge. I suggest the following process for making changes:

I will keep this PR open until Sunday so we have some time to move this into an issue. I hope this makes sense. In the meantime I'll make some time to review the proposed changes and help with an early review.

tmenguy commented 6 months ago

Hi,

I can remove the ICP/powerboost/schedule no pb, seeing the nature of the code was thinking would have been easier to review it directly, but no pb can remove. The last commit you’ve seen is coming from an authentication error I got, ok different from the timeout one, may need another issue again :( I’m in a business trip for the week, not sure I’ll have to work on it.

BR Thomas