lincomatic / open_evse

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

Bad temperature reading between 0oC and -0.5oC #110

Closed chris1howell closed 4 years ago

chris1howell commented 4 years ago

Hi, I have seen a problem with the OpenEVSE (firmware 5.0.1) with Wifi (firmware 2.8.1) when reporting temperature of -0.25oC. You can see from the attached picture of data logged to emoncms that there is systematically -256oC reading when the temperature crosses near 0oC. I see also the same from the OpenEVSE web page as you can see from two pictures taken a couple of seconds apart.

I've looked at the code here: https://github.com/OpenEVSE/open_evse/blob/stable/firmware/open_evse/open_evse.ino and it looks to me that the DS3231 detection mechanism at line 330 is the problem. The value of 0x3FF is a valid value of -0.25oC and does not indicates that the chip is not there. The I2C address ACK/NACK should be used for that purpose. Also, a valid check on the data would be a non-zero value in bits 0 to 5 of register 0x12, but that must be checked before shifting the byte and losing those bits of course. Another remark also regarding this code section. From the DS3231 datasheet, it is recommended to check the BSY bit before starting a conversion with the CONV bit and waiting for both bits to be cleared before reading the temperature.

thanks a lot, Jean-Pierre Lefebvre Capture d’écran_2019-11-16_09-33-37 Capture d’écran_2019-11-16_09-33-50 Capture d’écran_2019-11-17_09-42-36

lincomatic commented 4 years ago

this sounds pretty serious.. does the guy who made the suggestions have some code i can merge? it would save me a lot of time not having to read the spec sheet and re-writing the code. i didn't write the original functions

craigk46 commented 4 years ago

I’d like to fix this. The code change could take a few minutes. But I want to do it right by first re-creating the condition (freezing temperature) and then test the fix the same way. I also need to study if it potentially creates any unforeseen consequences. I have some time to work on this next week when I’m on vacation from work. I see that I really need to setup Arduino 1.6.15 instead of 1.8.1 to build OpenEVSE so I’ll take care to do that correctly.

Best Wishes, Craig K.

On Dec 15, 2019, at 7:44 PM, lincomatic notifications@github.com wrote:

 this sounds pretty serious.. does the guy who made the suggestions have some code i can merge? it would save me a lot of time not having to read the spec sheet and re-writing the code. i didn't write the original functions

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

lincomatic commented 4 years ago

Wow, glad to see that you're alive & well, @craigk46 . I was worried, since I hadn't heard from you in so long. You don't have to change your Arduino UI.. just go into the Tools->Board Manager, and you can pick which version of the AVR toolchain you want to use. Thanks for the help.

craigk46 commented 4 years ago

I made the fix and handled it is as a commit to open_evse.ino. I'm a bit rusty with GitHub so let me know if I did things correctly vs doing a pull request.