lincomatic / open_evse

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

Clear accumulated wattseconds prior to EVSE_STATE_DISABLED #63

Closed sandeen closed 7 years ago

sandeen commented 7 years ago

https://openev.freshdesk.com/support/discussions/topics/6000046121?page=1 has a report from a user that disabling and re-enabling the unit will cause the lifetime watt-hour display to increase by the last session's watt-second total on each disable/enable cycle.

This is because going into the disabled state does not clear the watt-second counter, and when we re-enable it and transition to EVSE_STATE_A, it will add g_WattSeconds to g_WattHours_accumulated each time.

Fix this by setting wattseconds to 0 prior to disabling the unit, similar to what is done prior to Sleep().

Signed-off-by: Eric Sandeen sandeen@sandeen.net

lincomatic commented 7 years ago

Have you tested this issue with the development branch? It seems like it's already been taken care of. In J1772EvseController::Update()

  #ifdef KWH_RECORDING          // Reset the Wh when exiting State A for any reason
    if (prevevsestate == EVSE_STATE_A) {
      g_WattSeconds = 0;
    }
  #endif
sandeen commented 7 years ago

I apologize, I don't have a great way to test all the state transitions yet. I think that code under Update() is in the released branch as well, though. Anyway, sorry for suggesting something that's untested, I'll just drop this for now if it doesn't look right.

sandeen commented 7 years ago

FWIW, I can reproduce the issue on 3.11.3

and observe the total watthours number increasing each time

lincomatic commented 7 years ago

Hmm. OK, sorry, I just tested this on the dev branch code, and you're right, it behaves the way you observed. The strange thing is, there was identical code to what you added to the Disable() function in the Sleep() function before, but I removed it due to a request from someone else, and I don't remember why. The CHANGELOG simply states that it was removed. There must have been some other bad side effect.

I think problem in the sleep case is that if you sleep/unsleep the EVSE during a charging session, then g_WattSeconds gets lost, so if you call $GU after the session is over, you're only getting the value since the last Enable().

This code was tricky for CraigK to implement, and don't want to mess it up, since I don't know all of the usage scenarios, so we have to tread carefully.

Can you think of a more bulletproof solution?