Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Safety check toggle RAPI protocol 4.0.1 support #145

Merged
merged 16 commits into from
Mar 7, 2018
Merged

Conversation

glynhudson
Copy link
Contributor

In RAPI protocol 4.0.1+ (openevse FW vD4.9.1 2017-08-11) the RAPI command for enable/disable the safety checks have changed. The new protocol is as follows:

From: https://github.com/lincomatic/open_evse/blob/development/firmware/open_evse/rapi_proc.h#L103

FF - enable/disable feature
 $FF feature_id 0|1
 0|1 0=disable 1=enable
 feature_id:
  D = Diode check
  E = command Echo
   use this for interactive terminal sessions with RAPI.
   RAPI will echo back characters as they are typed, and add a <LF> character
   after its replies. Valid only over a serial connection, DO NOT USE on I2C
  F = GFI self test
  G = Ground check
  R = stuck Relay check
  T = temperature monitoring
  V = Vent required check
 $FF D 0 - disable diode check
 $FF G 1 - enable ground check

This PR enables support for the new 4.0.1 RAPI API and removes safety check toggle support for older versions of RAPI API.

Questions

  • @jeremypoulter Should we be using the RapiSender class function to send these RAPI commands and then check for a successful OK?
  • @chris1howell The updated API now has support for enabling/disabling the temperature monitoring, should we add this as an option to toggle via the wifi-gateway? I think not, since temperature monitoring is so critical to openevse safety. I can't see a use case when we would need to turn it off via the wifi-gateway.
  • @chris1howell Should we try and maintain compatibility with older version of the RAPI API using the old (now deprecated API, see below)? I think since the change of API only effects enabling/disabling the tests (checking the status of the tests remains the same), it's acceptable to drop support for the old API. It just means users running older FW will have to resort to using the LCD menu to enable/disable the tests. No big deal. I'm keen to get the new API supported since the new non-tethered openevse unit we are working on does not have menu button therefore being able to toggle to tests via WiFi is important. All the non-tethered units will be shipped with the new FW.
  • @chris1howell we should have a separate discussion about doing a stable open_evse FW release. There are a number of important and useful fixes since the latest update. I have been running V4.11.0 for a couple of months now with no issues. The current stable release is 4.8.0

Old RAPI API

(DEPRECATED) SD 0|1 - disable/enable diode check
 $SD 0*0B
 $SD 1*0C
(DEPRECATED) SE 0|1 - disable/enable command echo
 $SE 0*0C
 $SE 1*0D
 use this for interactive terminal sessions with RAPI.
 RAPI will echo back characters as they are typed, and add a <LF> character
 after its replies. Valid only over a serial connection, DO NOT USE on I2C
(DEPRECATED) SF 0|1 - disable/enable GFI self test
 $SF 0*0D
 $SF 1*0E
(DEPRECATED) SG 0|1 - disable/enable ground check
 $SG 0*0E
 $SG 1*0F
SH kWh - set cHarge limit to kWh
SK - set accumulated Wh (v1.0.3+)
 $SK 0*12 - set accumulated Wh to 0
SL 1|2|A  - set service level L1/L2/Auto
 $SL 1*14
 $SL 2*15
 $SL A*24
SM voltscalefactor voltoffset - set voltMeter settings
(DEPRECATED) SR 0|1 - disable/enable stuck relay check
 $SR 0*19
 $SR 1*1A
ST starthr startmin endhr endmin - set timer
 $ST 0 0 0 0*0B - cancel timer
(DEPRECATED)SV 0|1 - disable/enable vent required
 $SV 0*1D
 $SV 1*1E

@glynhudson glynhudson changed the title Enable safety check toggle RAPI protocl 4.0.1 Safety check toggle RAPI protocol 4.0.1 support Feb 1, 2018
@jeremypoulter
Copy link
Collaborator

The RAPI HTTP endpoint "/r" should be using the RapiSender class, or are you asking about backward compatibility?
Have a look at https://github.com/OpenEVSE/ESP8266_WiFi_v2.x/blob/stable/src/web_server.cpp#L787:L795

@chris1howell
Copy link
Member

@chris1howell The updated API now has support for enabling/disabling the temperature monitoring, should we add this as an option to toggle via the wifi-gateway? I think not, since temperature monitoring is so critical to openevse safety. I can't see a use case when we would need to turn it off via the wifi-gateway.

Yeah, I agree. If it really needs to be off the command can be issued via RAPI.

@chris1howell Should we try and maintain compatibility with older version of the RAPI API using the old (now deprecated API, see below)? I think since the change of API only effects enabling/disabling the tests (checking the status of the tests remains the same), it's acceptable to drop support for the old API. It just means users running older FW will have to resort to using the LCD menu to enable/disable the tests. No big deal. I'm keen to get the new API supported since the new non-tethered openevse unit we are working on does not have menu button therefore being able to toggle to tests via WiFi is important. All the non-tethered units will be shipped with the new FW.

We should keep compatibility. It should be very easy with several optional. The first would be super easy, simply send both old and new commands. The unrecognized command would result in $NK. A better way would be to recognize the command that results in $NK and just send the correct command. The best option would be to read the RAPI version and issue the correct command for the version. I would be fine with any of the options.

@chris1howell we should have a separate discussion about doing a stable open_evse FW release. There are a number of important and useful fixes since the latest update. I have been running V4.11.0 for a couple of months now with no issues. The current stable release is 4.8.0

I am open to a release any time. I will compile 4.11.0 and test it.

@glynhudson glynhudson merged commit 1a87398 into master Mar 7, 2018
@glynhudson
Copy link
Contributor Author

Wops, I didn't mean to merge this. Was it ready? It tested with the new RAPI protocol 4.0.1+ and it worked well.

Have you tested the fallback to the older API? I don't have an open_evse controller running the older API to hand.

@jeremypoulter
Copy link
Collaborator

jeremypoulter commented Mar 7, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants