nordicopen / pyeasee

Easee EV charger API python library
MIT License
39 stars 11 forks source link

Charger.set_dynamic_charger_circuit_current extended #58

Closed perjarne closed 2 years ago

perjarne commented 2 years ago

Updated code for how to set dynamic current limit on circuit level. Affects two methods: Charger.set_dynamic_charger_circuit_current New optional parameter, timeToLive has been added.

Circuit.set_dynamic_current New optional parameter, timeToLive has been added.

timeToLive specifies, in minutes, the validity of the new current limit. If timeToLive is set to zero, then the setting is valid until the next time it is changed, or the charger is restarted. This call is used: https://api.easee.cloud/api/sites/siteId/circuits/circuitId/dynamicCurrent If timeToLive is omitted, then the old code will be executed. This means that the new current limit will be valid until the next time it is changed, a new charging sessions starts, or the charger is restarted, whichever comes first.

API documentation has been updated accordingly

Hellowlol commented 2 years ago

Hi! This looks like a nice an simple PR but there some unnecessary repetition and a print statement.

perjarne commented 2 years ago

Hi! This looks like a nice an simple PR but there some unnecessary repetition and a print statement.

Thanks for feedback. It was clumsy to leave the print statements. I will remove them. Regarding the repetition, I take it you are referring to the old code I left in Circuit.set_dynamic_current. I was in two minds here. The two API calls don't work exactly in the same way so I kept the old one for backwards compatibility. My thinking was that perhaps someone is using this function under the assumption that the current limit will be reset whenever a car is connected. In the new API, timeToLive = None has the same effect as timeToLive = 0, which means that the setting survives car connect/disconnect and is valid until changed again. An alternative is of course to always use the new API. Perhaps this is the best option, and clarify in the documentation how the call works so there are no surprises.

olalid commented 2 years ago

I would prefer to just call the new API, but I am not sure how you came to the conclusion that timeToLive = 0 is the same as calling the old API, I see nothing in the documentation that indicates that it is so: https://developer.easee.cloud/reference/post_api-sites-siteid-circuits-circuitid-dynamiccurrent

perjarne commented 2 years ago

This commit:

  1. Removed print statements
  2. Removed call to /api/sites/siteId/circuits/circuitId/settings

To summarize the new behavior implemented in this branch: Updated code for setting dynamic current limit on circuit level. Affects two methods: Charger.set_dynamic_charger_circuit_current New optional parameter, timeToLive has been added.

Circuit.set_dynamic_current New optional parameter, timeToLive has been added.

timeToLive specifies, in minutes, the validity of the new dynamic current. If timeToLive is set to zero or omitted, then the new dynamic current is valid until the next time it is changed, or the charger is restarted. This call is used: https://api.easee.cloud/api/sites/siteId/circuits/circuitId/dynamicCurrent

The documentation has been updated accordingly.

olalid commented 2 years ago

timeToLive specifies, in minutes, the validity of the new dynamic current. If timeToLive is set to zero or omitted, then the new dynamic current is valid until the next time it is changed, or the charger is restarted. This call is used: https://api.easee.cloud/api/sites/siteId/circuits/circuitId/dynamicCurrent

Again, where are you seeing this being documented?

perjarne commented 2 years ago

timeToLive specifies, in minutes, the validity of the new dynamic current. If timeToLive is set to zero or omitted, then the new dynamic current is valid until the next time it is changed, or the charger is restarted. This call is used: https://api.easee.cloud/api/sites/siteId/circuits/circuitId/dynamicCurrent

Again, where are you seeing this being documented?

What I mean is that I have updated the HTML files to reflect the changes I have made to the method call.

olalid commented 2 years ago

timeToLive specifies, in minutes, the validity of the new dynamic current. If timeToLive is set to zero or omitted, then the new dynamic current is valid until the next time it is changed, or the charger is restarted. This call is used: https://api.easee.cloud/api/sites/siteId/circuits/circuitId/dynamicCurrent

Again, where are you seeing this being documented?

What I mean is that I have updated the HTML files to reflect the changes I have made to the method call.

Is my question not clear? You state "If timeToLive is set to zero, then the setting is valid until the next time it is changed". I am asking you, where did you get that information from? The API documentation that Easee provides states no such thing as far as I am able to see. If it is the case that you have found it out by trial and error, that is completely fine, but if that is the case I want to contact Easee to verify that this is actually something they intentionally support, otherwise it could break in any if their next updates. I just want to make sure we are not doing anything based on assumptions here. I like your intentions here, do not get me wrong, I just want to make sure we do it the right way.

PS. The HTML files are auto-generated, if you want to change what is written there, you need to change the comment first in each function in the code itself, that is where that information is collected from. Any changes you do directly in the HTML-files will be overwritten as part of the release process.

perjarne commented 2 years ago

timeToLive specifies, in minutes, the validity of the new dynamic current. If timeToLive is set to zero or omitted, then the new dynamic current is valid until the next time it is changed, or the charger is restarted. This call is used: https://api.easee.cloud/api/sites/siteId/circuits/circuitId/dynamicCurrent

Again, where are you seeing this being documented?

What I mean is that I have updated the HTML files to reflect the changes I have made to the method call.

Is my question not clear? You state "If timeToLive is set to zero, then the setting is valid until the next time it is changed". I am asking you, where did you get that information from? The API documentation that Easee provides states no such thing as far as I am able to see. If it is the case that you have found it out by trial and error, that is completely fine, but if that is the case I want to contact Easee to verify that this is actually something they intentionally support, otherwise it could break in any if their next updates. I just want to make sure we are not doing anything based on assumptions here. I like your intentions here, do not get me wrong, I just want to make sure we do it the right way.

PS. The HTML files are auto-generated, if you want to change what is written there, you need to change the comment first in each function in the code itself, that is where that information is collected from. Any changes you do directly in the HTML-files will be overwritten as part of the release process.

OK, I misunderstood your question. The statement about how "timeToLive = 0" is interpreted is coming from Easee. I asked the question in their forum and here is the response from the admin: https://developer.easee.cloud/discuss/618402c760630f0494a064f1

olalid commented 2 years ago

Ok, good, I guess that is enough. But can you please change the documentation as I indicated above?

olalid commented 2 years ago

Thanks @perjarne, this change is now included in the the latest release.