serkri / SmartEVSE-3

Smart Electric Vehicle Charging Station (EVSE)
MIT License
68 stars 28 forks source link

modem/EV state extensions: add extra ISO value support, basic AutoCharge #160

Closed arpiecodes closed 1 year ago

arpiecodes commented 1 year ago

This pull request builds on and refactors the excellent modem support contributions by ArendJanKramer.

Not all cars support communicating the added values through ISO, which is why it's largely coded in such a way that missing values would also work. For example, calling /ev_state with only a current_soc would still update the current SoC, as long as the rest of the values are -1. It is also possible to supply a 'fixed' energy_capacity this way. In theory, this would also open up possibilities for integrating SoC data sourced from third party app data (like BMW Connected Drive).

PS: I also removed BulkSoC as this would only matter for fast charging (which we can't do with the SmartEVSE). I think the car will give a lower FullSoC if there is a setting active that restricts the max amount of charge for whatever reason.

There's also a PyPLC pull request that updates the code there to extract and supply the new values from the ISO modem. The changes in that pull request are required to make the changes in this pull request work, see https://github.com/uhi22/pyPLC/pull/8.

Code works on my machine, so I'd definitely appreciate some tests before we consider merging this. This is why I've opened it as a Draft pull request.

Screenshot 2023-06-26 at 18 34 25

Screenshot 2023-06-26 at 18 34 29

dingo35 commented 1 year ago

@ArendJanKramer @arpiecodes Ok guys, thanks for all the great work. I just found a small bug that was introduced in 86b7ed4022a : when switching to OFF, Connected switches to YES, even when vehicle is not connected. Also happens if Modem == Not present. Suspect that somewhere PWM is set to 0% so the pilot is no longer detected.

Is this a simple bug we can tackle in this PR or we do this separately?

ArendJanKramer commented 1 year ago

@ArendJanKramer @arpiecodes Ok guys, thanks for all the great work. I just found a small bug that was introduced in 86b7ed4 : when switching to OFF, Connected switches to YES, even when vehicle is not connected. Also happens if Modem == Not present. Suspect that somewhere PWM is set to 0% so the pilot is no longer detected.

Is this a simple bug we can tackle in this PR or we do this separately?

Interesting, I didn't notice but I ran into issues with reliably determining disconnected state; hence the disconnectEvent. Since the CP is disconnected in OFF you can't really measure indeed.

I suspect the bug happens because SetAccess(0) calls CP_OFF, killing the sensing. On one hand, we can get rid of that, but on the other hand I expect turning something off and on again that it works the same as re-plugging the car.

Let me try some things in a bit

dingo35 commented 1 year ago

Sure, take your time. But please note that mode "OFF" equals the access bit set to zero, meaning that access is denied and has to be granted by RFIDreader, API, or (new) EVCCID match with setting/profile.

So conceptually that doesnt mean that CP is disconnected; so in my view setaccess(0) should not call CP_OFF.

I would expect CP to be on any value except 0, so connect/disconnect can be sensed. If connect is sensed AND modem present then set pwm=5% to start modem comm.

Unless necessary in some step of the modem comm, CP_OFF should never be called?!?

arpiecodes commented 1 year ago

I suppose the issue here is that without CP_OFF it is pretty hard to not confuse the car when cycling through the charge phases (e.g. it's the better way to reset communication as well). I think we need to properly implement the PWM states according to spec and identify the correct phase to "idle in" without it granting access.

My car is pretty finicky about going through those phases in proper sequence, and else simply calls it a day and requires a re-plug before it will do anything again. It happened quite a few times already where even a CP_OFF/reset through 10%/100% etc did not do anything and I really had to go out and re-plug.

dingo35 commented 1 year ago

Exactly! Access bit = off corresponds to state A = standby = CP=12V ... https://en.m.wikipedia.org/wiki/SAE_J1772

ArendJanKramer commented 1 year ago

I think we have two options:

It's important to disconnect before charging if you have a modem, otherwise you can't switch between off/normal with modem readout. The second fix is more involved than the first obviously.

I'd say we can merge this PR, and put this bugfix it into another. The intermediate solution is quite simple though, just add an if statement to the setAccess, it'll be funky for the three of us using the modem, but we can continue work without bothering the rest. I'm happy to make the fix.

dingo35 commented 1 year ago

Ok, I will merge tomorrow! Thx for all the work!

dingo35 commented 1 year ago

Ok merged it all but something went wrong somewhere; I found a missing } and a superfluous

in index.html, but still layout is messed up: Schermafdruk van 2023-06-30 12-42-58

As you see Phase Details (Original) show up, and I have no home battery, so Phase Details should pop up, as also my EVMeter details.

I corrected the behaviour by only giving out ev_data when Modem present (see 1ce497fd169e), but when I set modem to experimental the problem is still there.

@arpiecodes Perhaps I did something wrong with merging the PR's, could you guys diff with your (working) versions to see where the problem is? Have spent hours but cannot find it....

dingo35 commented 1 year ago

@ArendJanKramer any progress on the connected/disconnected issue? Im already getting bug reports on this #165

arpiecodes commented 1 year ago

@arpiecodes Perhaps I did something wrong with merging the PR's, could you guys diff with your (working) versions to see where the problem is? Have spent hours but cannot find it....

I'll compile it and try it out soon. Nothing of interest in the developer tools console?

Usually when this happened to me the json doc char array initialised inside the /settings endpoint handler was too small to fit all of the data, which confusingly doesn't make it fail but does make it incomplete. I bet with MQTT and the new Modem suff combined 1500 (or even 1600) could be a tight fit.. Maybe we should move the MQTT settings to a separate endpoint.

dingo35 commented 1 year ago

Only one message: Uncaught TypeError: data.phase_currents.original_data is undefined

...which makes sense because I have no home_battery installed .....

I increased that doc to 1600 but that didnt change anything; I tried to get rid of the doc/serialize stuff by using some macro's but it increases ram usage a lot, that serialize stuff is pretty efficient.....

arpiecodes commented 1 year ago

Only one message: Uncaught TypeError: data.phase_currents.original_data is undefined

...which makes sense because I have no home_battery installed .....

I increased that doc to 1600 but that didnt change anything; I tried to get rid of the doc/serialize stuff by using some macro's but it increases ram usage a lot, that serialize stuff is pretty efficient.....

OK, then it's just JS code that bugs out.

Put the parts where it tries to use data.phase_currents.original_data.* values in between an if(data.phase_currents.original_data) block. It will make the error go away and fix the rendering.