lincomatic / open_evse

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

RAPI response has no checksum #53

Closed presslab-us closed 8 years ago

presslab-us commented 8 years ago

There doesn't seem to be any checksum ability on the RAPI response packet. Maybe this was deemed unnecessary as the responses won't cause potential hazards like a bad command could. I'm just wondering what the thoughts on this are; if I added a way to optionally turn on a response checksum (XOR) would that be desired?

lincomatic commented 8 years ago

This is a good idea. It was an oversight on my part not to add a checksum to the response. Unfortunately, to avoid breaking existing code, as you mentioned, it should be optional, so I guess we need a command to enable/disable it.

lincomatic commented 8 years ago

The latest dev release (D4.0.0) now appends an XOR checkbyte to all RAPI responses. At the risk of breaking existing code, I decided to just enable it by default. It will take too much code space to add a command to disable it, so it can only be disabled by commenting out #define RAPI_RESPONSE_CHK.

Thanks again for bringing this glaring omission to my attention.

presslab-us commented 8 years ago

Great! I will test it with my ZigBee module and let you know how it goes.

lincomatic commented 8 years ago

I noticed another omission today.. the async messages $ST and $WF were also missing check bytes. I just pushed vD4.1.0, which adds them. .. new RAPI version is 2.0.1

presslab-us commented 8 years ago

I updated my ZigBee code to work with the checksum, and everything worked as expected. Thanks.