search

lincomatic / open_evse

Firmware for Open EVSE
GNU General Public License v3.0
114 stars 163 forks source link

No RAPI API for enabling/disabling the temperature checking #69

Closed jeremypoulter closed 7 years ago

jeremypoulter commented 7 years ago

I do not see an API for enabling/disabling the temperature monitoring. Can one be added?

lincomatic commented 7 years ago

I modified the code so that $S0 0 0 disables the temperature checking

https://github.com/lincomatic/open_evse/commit/c7508775eaa8e9efd6f3128f583565787df74872

jeremypoulter commented 7 years ago

And to enable again I can call $SO with the values retrieved from $GO, right?

lincomatic commented 7 years ago

right

jeremypoulter commented 7 years ago

The $SO and $GO commands are set protected by TEMPERATURE_MONITORING_NY, which is not defined by default.

// not yet #define TEMPERATURE_MONITORING_NY

What is the downside of this? Can there be a more explicit enable/disable command for the temp monitoring as with the other tests?

lincomatic commented 7 years ago

OK, I completely messed this one out, by adding the code to TEMPERATURE_MONITORING_NY code, which is never enabled, because it's half-baked. I tried to jam in the enable/disable functionality in order to save space. Going forward, are there other features that need enable/disable? Though it won't be backward compatible, I wonder if rather than adding a RAPI command for each feature, we should just use one command. For instance: $FF feature onoff What do you think?

jeremypoulter commented 7 years ago

No worries, I think that could work, would you remove all the other enable/disable commands?

lincomatic commented 7 years ago

It would be nice to remove all of other enable/disable commands, except that it would break existing code. I suppose the client code could just test for $FF and if it's there, then assume the other ones are gone. What do you think?

jeremypoulter commented 7 years ago

Could do, maybe also increment the protocol version as well?

lincomatic commented 7 years ago

OK, I implemented $FF to replace the existing $SD/SE/SF/SG/SR/SV commands. it includes a subfunction to enable/disable temperature check. The RAPI protocol version is 4.0.0 when it's enabled.

https://github.com/lincomatic/open_evse/commit/77fcecf0096bacb449ca31d32509c27dd1a70d14