dingo35 / SmartEVSE-3.5

Smart Electric Vehicle Charging Station (EVSE)
MIT License
38 stars 13 forks source link

Rework Set_Nr_of_Phases_Charging() ? #57

Open cvanlith opened 2 months ago

cvanlith commented 2 months ago

The current method Set_Nr_of_Phases_Charging() seems to have a shaky design. If no EV-meter is present and EnableC2 is AUTO the result is always 3. This is unfortunate as with EnableC2 on AUTO, the number of phases is meant to change. Set_Nr_of_Phases_Charging() is only used once in the code and only if EnableC2 is AUTO. So the methode is only used in a case where the outcome is always 3. So, there is no use for the method without EV-meter.

I would like to propose another route for Set_Nr_of_Phases_Charging(). Use Force_Single_Phase_Charging() in combination with Switching_To_Single_Phase to calculate the number of phases the charging is supposed to be done with (there is no certainty that it actually does). Next you could use the EV-meter to check if the calculated number of phases is found in reality and adjust if necessary. If you do not have an EV-meter, you simply use the calculated number of phases. To my opinion the route gives the best estimate of the number of the charging phases for systems with and without an EV-meter.

Can anymore comment on this? Am I missing something here? I will be pleased to write up an alternative but like to know is this is the right route. Thanks.

dingo35 commented 2 months ago

Hi Chris, thx for working on this problem; you are absolutely right that the current logic is not very good; it is a remnant of much more complicated logic we decided to get rid of because it didn't work (the famous "phase detection logic"), but thought it would be ashame to lose this part of the logic.

"The current method Set_Nr_of_Phases_Charging() seems to have a shaky design. If no EV-meter is present and EnableC2 is AUTO the result is always 3. This is unfortunate as with EnableC2 on AUTO, the number of phases is meant to change. Set_Nr_of_Phases_Charging() is only used once in the code and only if EnableC2 is AUTO. So the methode is only used in a case where the outcome is always 3. So, there is no use for the method without EV-meter."

That is correct; I don't see any way we could detect the phases without using an EV-meter, and the default/safe value to fall back on is 3 phases. Note that we only need to know the number of phases in Solar mode. In Smart mode, we calculate all currents PER PHASE. So a 1phase grid feeding a 1phase EV is fine, a 3phase grid feeding a 3phase EV is find, only a 3phase grid feeding a 1phase EV is unnecessary cautious because it takes also into account the 2 phases that are not being used to charge on. This is not a big problem, since: -in an overload situation of one of the 2 phases, the SmartEVSE would (unnecesarily) reduce charging and eventually shut down; however, this situation should be extremely rare, and the problems of a phase overloading would be much bigger then a SmartEVSE shutting down... -in an underload situation you would not use the entire capacity of the grid; this is the current behaviour of the SmartEVSE.

In Solar mode the problem is that Isum and MaxImport are summed over all phases, and now it becomes important to know how many phases we should involve in our calculations.

"I would like to propose another route for Set_Nr_of_Phases_Charging(). Use Force_Single_Phase_Charging() in combination with Switching_To_Single_Phase to calculate the number of phases the charging is supposed to be done with (there is no certainty that it actually does). Next you could use the EV-meter to check if the calculated number of phases is found in reality and adjust if necessary. If you do not have an EV-meter, you simply use the calculated number of phases. To my opinion the route gives the best estimate of the number of the charging phases for systems with and without an EV-meter."

Why not choosing for the obvious: if (EVMeter) measure with Set_Nr_of_Phases_Charging else guess conservatively with Force_Single_Phase_Charging()

obviously the names should be adapted to more logical names....

EDIT edited the if - else clauses....

cvanlith commented 2 months ago

"Why not choosing for the obvious: if (EVMeter) measure with Set_Nr_of_Phases_Charging else guess conservatively with Force_Single_Phase_Charging()"

I think you also need Switching_To_Single_Phase.

I will write up some new code to investigate further and let you know.

dingo35 commented 2 months ago

Great, looking forward to it!