Delay end time not stopping charge

glynhudson commented 6 years ago

After updating from 4.11.0 to latest version (4.12.3) the delay timer end charge time does not work. Starting a charge using the delay timer works fine, however the charge does not stop as it should at the allocated end time.

Over the weekend I will try and debug establish after what version this bug was introduced, sorry not had much time to debug today. This is just an interim post to establish if anyone else can re-create this issue?

The delay timer was set using RAPI via the wifi-gateway and was verified to be correct on the LCD and using $GD.

lincomatic commented 6 years ago

I've been running 4.11.2 for a while, and it works on timer.. set from the front panel. I looked at the changes in 4.11.3, and they shouldn't affect the timer. I will try 4.11.3 on my test unit

lincomatic commented 6 years ago

What's the code size and RAM usage of your build of 4.12.3? Mine with Arduino IDE 1.8.3 and Arduino AVR boards by Arduino 1.6.15:

Sketch uses 31544 bytes (96%) of program storage space. Maximum is 32768 bytes. Global variables use 1538 bytes (75%) of dynamic memory, leaving 510 bytes for local variables. Maximum is 2048 bytes.

lincomatic commented 6 years ago

I just tested 4.11.3, and the timer works fine. Stops when it's supposed to.

glynhudson commented 6 years ago

Mm that's interesting. I'll do some more testing tomorrow. I replicated the issue twice today. Once overnight when my EV should have stopped charging at the end of off peak this morning then once this afternoon to verify. I've been using delay timer every night for pretty much the last 12 months and it's never failed before.

I will test on my bench setup when I'm in the office on Monday. Thanks a lot for testing.

What's the code size and RAM usage of your build of 4.12.3?

Program:   31048 bytes (94.8% Full)
(.text + .data + .bootloader)

Data:       1542 bytes (75.3% Full)
(.data + .bss + .noinit)

How can I measure the RAM usage? What is your build size?

lincomatic commented 6 years ago

I posted it above. Data is the static RAM usage. So the rest is all the stack has left. You only have 4 bytes more than me.. shouldn't be a problem, unless we're that close to stack overflow. Are you using the standard build, or compiling your extra code into it? Try it w/o any extra code

glynhudson commented 6 years ago

I've tested with the latest development branch directly from your repo compiled using Arduino and I'm still able to replicate the issue. Going back to 4.11.0 and the stop timer works again.

Tomorrow I will do some more testing to try and establish exactly what version introduced this issue I'm experiencing. It's strange your not able to replicate. See attached the .hex file I compiled directly from the dev repo. Can you replicate the issue using this .hex file? upstream-dev-arduino.zip

glynhudson commented 6 years ago

From my testing I've determined that the bug (at least that I'm experiencing: delay timer not stopping at the correct time) was introduced in commit various delay timer behavioural fixes 6cc9610c8390a61aa563e35102b30f5bc2394ba7

The timer works when going back one commit to fix compile error 676e25595e347217b6499bd249a4125bf496ab7a

I've had a look through the change in commit various delay timer behavioural fixes, I can't see anything obvious. But this commit contains a lot of changes to do with delay timers. I'm not totally sure what exactly all the changes in the commit are for.

Are you able to replicate the issue?

I'm testing with:

compiling with Arduino 1.8.5
Using RAPI via wifi-gateway to set delay timer
EV is connected when delay timer is set

lincomatic commented 6 years ago

Does it always happen, no matter the time intervals? Only when they're a certain length or cross midnight? Let me know the start/stop times you are using. I am using the same code to charge my car every night midnight->6am

glynhudson commented 6 years ago

I've never tested crossing midnight. All my tests have either been during the early hours of the morning .eg. 4am-5am or testing for 1min duration usually in late evening e.g. 21:49 > 21:50

lincomatic commented 6 years ago

Is the bug reproducible via the front panel pushbutton, or only via the WiFi client? If it's reproducible via front panel, please give me the start & end times that trigger the bug. If it's reproducible via RAPI only, I need the exact sequence of RAPI commands

glynhudson commented 6 years ago

When running version: https://github.com/lincomatic/open_evse/commit/6cc9610c8390a61aa563e35102b30f5bc2394ba7

Steps to replicate issue (via RAPI):

Delay timer ON time was set to 12:44 and off time to 12:45 using RAPI command:

$ST 12 44 12 45

returned responce:

$OK^20

Delay timer set was verified using $GD with responce

$OK 12 44 12 45^21

The delay timer setting was also verified by checking the correct time setting on the LCD

The RAPI commands were set via HTTP using wifi-gateway e.g.

http://openevse/r?json=1&rapi=$ST+12+44+12+45

The EV was connected and the EVSE was in sleeping mode when the delay timer was set

Result:

EVSE switched on and started charging at 12:44 but did not switch off at 12:45.

The same issue occurs with any time period I choose.

Delay timer works fine as intended when running https://github.com/lincomatic/open_evse/commit/676e25595e347217b6499bd249a4125bf496ab7a

Via LCD display

The delay timer was cleared using $ST 0 0 0 0 then set using the LCD button interface , the same issuer was observed.

lincomatic commented 6 years ago

OK, I was able to reproduce it. I see the problem lies in a change in the delay timer logic that I made. In various delay timer behavioural fixes 6cc9610, I changed the logic so that if the EV was still charging at the end of the delay timer charging interval, it would wait until the EV switched back to connected state (STATE B) before going back to sleep. This allows the EV to get fully charged, even if the charging time window is too small.

I can now see that, this is not always the desired behavior. I will need some time to figure out if taking out this behavior breaks any of the other changes before I can come up with a proper fix.

glynhudson commented 6 years ago

Sure, let me know if I can help.

I've read the changelog for the changes made to the delay timer, I can understand the thinking behind the override delay timer > charge to full, however I would argue the charge should always stop at the delay timer end time.

I've gone back to running 4.11.0 and IMO the delay timer works flawlessly:

It's possible to override the delay timer using button/RAPI to start a charge outside of the delay timer timeframe.
If delay timer start time is overridden to start before the start time the charge still finishes at the end time. I would argue that this is desirable operation since the end time should always mean end. If the user wasn't to continue charging the delay timer could be removed. IMHO it's confusing if the EV does not aways stop charging at the end time.
During a charge session started using delay timer it;s possible to override pause using the button / RAPI.

lincomatic commented 6 years ago

Yes, I agree that the end time should generally mean the end. But coming back to a car that's only partially charged when you have somewhere to go can really suck. I had a problem where I plugged in the car near the cutoff time of the timer, and then the car was only partially charged. Luckily, I had a gas car to use instead. Yes, I know you can turn off the timer, but it's inconvenient, and then you need to remember to turn it back on. Also, except in this particular case, quick pressing the front button when it's sleeping generally wakes it up long enough to get a full charge, so you generally expect that behavior.

However, I agree, most people would expect the end time to be the end. I will change it back.

glynhudson commented 6 years ago

Thanks a a lot. Sure, I can understand that must have been annoying. However, I still think end should mean end. Using the wifi interface it's very quick and easy to remove / adjust the timer. My schedule is irregular therefore I adjust the charging timer most days using the wifi interface. It's especially nice using an Android device with the android time picker: https://github.com/OpenEVSE/ESP8266_WiFi_v2.x/raw/master/docs/mobile-clock.png.

That said I would be be open to the override button removing the charging timer altogether if you were really keen on this. The bug in question is not directly to do with the override button function. The issue is that even if the override button is not pressed the charge does not end at the designated end time making the delay timer not functional.

Please let me know if I can help.

lincomatic commented 6 years ago

It was just a one line change.

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

glynhudson commented 6 years ago

Works great, thanks a lot 👍

lincomatic / open_evse