Hi Think I found out why I was unable to send mail to this list. (wrong sender address) My 2016 Leaf still has flapping SOC. I made the following changes in vehicle_nissanleaf.cpp and compiled the code. case 0x50d: //Changed this from 5bc. 50d is the instrument SOC { uint16_t nl_gids = ((uint16_t) d[0] << 2) | ((d[1] & 0xc0) >> 6); if (nl_gids == 1023) { break; } uint16_t max_gids = MyConfig.GetParamValueInt("xnl", "maxGids", GEN_1_NEW_CAR_GIDS); float km_per_kwh = MyConfig.GetParamValueFloat("xnl", "kmPerKWh", GEN_1_KM_PER_KWH); float wh_per_gid = MyConfig.GetParamValueFloat("xnl", "whPerGid", GEN_1_WH_PER_GID); m_gids->SetValue(nl_gids); StandardMetrics.ms_v_bat_soc->SetValue(nl_gids / 4); //Changed this so the value is divided by 4. SOC now shows exactly the same in the app as the instrument-panel does, and is not flapping. (I know the instrument-panel is different from the true SOC) My coding capabilities are limited so if someone has a better idea, I can test it on my car. And now to a different thing: The app supports unlocking and locking of doors. The feature doesn't seem to be implemented for the Nissan Leaf. I have all the CAN-codes needed to make this work. Please let me know if I can contribute. Kind regards, Stein Arne Sordal
On 02/05/18 01:08, ovms wrote:
Think I found out why I was unable to send mail to this list. (wrong sender address) My 2016 Leaf still has flapping SOC.
Do you have a 30kWh battery?
I made the following changes in vehicle_nissanleaf.cpp and compiled the code.
*case 0x50d**:* //Changed this from 5bc. 50d is the instrument SOC
{
uint16_t nl_gids = ((uint16_t) d[0] << 2) | ((d[1] & 0xc0) >> 6);
if (nl_gids == 1023)
{
break;
}
uint16_t max_gids = MyConfig.GetParamValueInt("xnl", "maxGids", GEN_1_NEW_CAR_GIDS);
float km_per_kwh = MyConfig.GetParamValueFloat("xnl", "kmPerKWh", GEN_1_KM_PER_KWH);
float wh_per_gid = MyConfig.GetParamValueFloat("xnl", "whPerGid", GEN_1_WH_PER_GID);
m_gids->SetValue(nl_gids);
StandardMetrics.ms_v_bat_soc->SetValue(*nl_gids / 4*); //Changed this so the value is divided by 4.
SOC now shows exactly the same in the app as the instrument-panel does, and is not flapping. (I know the instrument-panel is different from the true SOC)
Right, so you've confirmed that if module sends a reasonable SOC to the server, the value stops flapping in the app?
My coding capabilities are limited so if someone has a better idea, I can test it on my car.
We can make the SOC calculation configurable so that you can choose to use the instrument panel SOC or an SOC based on the new car battery size. When I took over the leaf support I wanted to use the instrument panel soc too, see Tom Saxton's reply which convincingly dissuaded me wanting to switch: http://lists.openvehicles.com/pipermail/ovmsdev/2016-May/002993.html (this discussion that was before I knew SOC was available so I had some no longer relevant ideas about how to actually implement it). Now we have the easier to configure v3 platform and more users wanting to use the instrument panel SOC I'll make it configurable. I should be able to get that done tonight.
And now to a different thing:
The app supports unlocking and locking of doors. The feature doesn't seem to be implemented for the Nissan Leaf.
I have all the CAN-codes needed to make this work. Please let me know if I can contribute.
I'm uncomfortable doing this on my own car, but I'm very happy to have the feature implemented. I think it should be hidden behind a configuration option, so you can choose whether the standard functionality allows the car to be unlocked, but that is a different and not Leaf specific conversation. You should be able to implement it quite easily by following the pattern used in CommandStartCharge() in the Leaf code and the Lock and UnLock implementations in the Tesla Roadster and Kia Soul code. Be sure to send the frame to the correct CAN bus. The console logging functions are very useful for confirming what is going on when you're developing. Thanks for
On Wed, May 02, 2018 at 07:39:57AM +1200, Tom Parker wrote:
We can make the SOC calculation configurable so that you can choose to use the instrument panel SOC or an SOC based on the new car battery size. When I took over the leaf support I wanted to use the instrument panel soc too, see Tom Saxton's reply which convincingly dissuaded me wanting to switch: http://lists.openvehicles.com/pipermail/ovmsdev/2016-May/002993.html (this discussion that was before I knew SOC was available so I had some no longer relevant ideas about how to actually implement it).
Now we have the easier to configure v3 platform and more users wanting to use the instrument panel SOC I'll make it configurable. I should be able to get that done tonight.
This picks up on the same discussion we started earlier when I tried to make the same change. But we seem to be arguing as if there has to be one ideal metric, when we really need several of different metrics for different purposes. There is utility in having a vehicle metric for the dash SOC% because that is how the car reports on progress during charging---it's going to stop at 100% (or 80%) and slow down in a predictable way at the number increases. There is also utility in having another vehicle metric which represents the amount of available energy in the battery---though I see no good reason why that should be expressed as a percentage of some arbitrary number I've had to set in the first place; it seems fundamentally better to use a standard unit like kWh. Is there a metric for that already? Once we know available energy, we can derive other useful things like estimated range, and those calculations can be vehicle-independent, because they just use values in standard units like kWh, W/km etc. It seems much more reasonable for this to be where the user can choose to apply their own scaling preferences, rather than having that in the code for any specific vehicle.
And now to a different thing:
The app supports unlocking and locking of doors. The feature doesn't seem to be implemented for the Nissan Leaf.
I have all the CAN-codes needed to make this work. Please let me know if I can contribute.
I'm uncomfortable doing this on my own car, but I'm very happy to have the feature implemented. I think it should be hidden behind a configuration option, so you can choose whether the standard functionality allows the car to be unlocked, but that is a different and not Leaf specific conversation.
Yes, I'm also not sure I would always want this enabled, so it would be good to have a config option to enable it, but it's something I would like to help work on.
My 2c: This seems to be a hot topic amongst Nissan Leaf owners. Am I correct in that the displayed SOC doesn’t show degradation? In general, OVMS has tried to remotely show exactly what was on the car dashboard. We went to some extreme lengths in the original Tesla Roadster code to meet that requirement (for example the km/miles conversions were not accurate in the car implementation, so we tried to mimic the car’s inaccurate algorithm in our approach). Absolutely no objection to adding vehicle specific metrics. A GID value (for Nissan Leaf) is the obvious example, but also other indications of battery health. We have SOC%, SOH%, and CAC currently as global metrics. Regards, Mark.
On 2 May 2018, at 6:41 AM, Robin O'Leary <ovmsdev@caederus.org> wrote:
On Wed, May 02, 2018 at 07:39:57AM +1200, Tom Parker wrote:
We can make the SOC calculation configurable so that you can choose to use the instrument panel SOC or an SOC based on the new car battery size. When I took over the leaf support I wanted to use the instrument panel soc too, see Tom Saxton's reply which convincingly dissuaded me wanting to switch: http://lists.openvehicles.com/pipermail/ovmsdev/2016-May/002993.html (this discussion that was before I knew SOC was available so I had some no longer relevant ideas about how to actually implement it).
Now we have the easier to configure v3 platform and more users wanting to use the instrument panel SOC I'll make it configurable. I should be able to get that done tonight.
This picks up on the same discussion we started earlier when I tried to make the same change. But we seem to be arguing as if there has to be one ideal metric, when we really need several of different metrics for different purposes.
There is utility in having a vehicle metric for the dash SOC% because that is how the car reports on progress during charging---it's going to stop at 100% (or 80%) and slow down in a predictable way at the number increases.
There is also utility in having another vehicle metric which represents the amount of available energy in the battery---though I see no good reason why that should be expressed as a percentage of some arbitrary number I've had to set in the first place; it seems fundamentally better to use a standard unit like kWh. Is there a metric for that already?
Once we know available energy, we can derive other useful things like estimated range, and those calculations can be vehicle-independent, because they just use values in standard units like kWh, W/km etc. It seems much more reasonable for this to be where the user can choose to apply their own scaling preferences, rather than having that in the code for any specific vehicle.
And now to a different thing:
The app supports unlocking and locking of doors. The feature doesn't seem to be implemented for the Nissan Leaf.
I have all the CAN-codes needed to make this work. Please let me know if I can contribute.
I'm uncomfortable doing this on my own car, but I'm very happy to have the feature implemented. I think it should be hidden behind a configuration option, so you can choose whether the standard functionality allows the car to be unlocked, but that is a different and not Leaf specific conversation.
Yes, I'm also not sure I would always want this enabled, so it would be good to have a config option to enable it, but it's something I would like to help work on. _______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
On Wed, May 02, 2018 at 08:14:24AM +0800, Mark Webb-Johnson wrote:
This seems to be a hot topic amongst Nissan Leaf owners. Am I correct in that the displayed SOC doesn’t show degradation?
Yes, 2nd gen and later models have a numeric 0–100% SOC meter, which does not account for degradation---there was a separate 12-blob "health" gauge for that, though that's gone on the 2018 model. I think a lot of this stems from the fact that the 1st gen didn't have any numeric SOC display; they had only a 12-blob "fuel" gauge. Indeed, it was this lack of precision that sent many of us rummaging on the CAN bus to find better values. As there was no documentation and no standard set by the manufacturer, conventions arose piecemeal as new nuggests were discovered, which gave us GIDs and the SOC meter relative to original battery size (which may not have seemed so arbitrary when there was was only one original battery size).
In general, OVMS has tried to remotely show exactly what was on the car dashboard. We went to some extreme lengths in the original Tesla Roadster code to meet that requirement (for example the km/miles conversions were not accurate in the car implementation, so we tried to mimic the car’s inaccurate algorithm in our approach).
Absolutely no objection to adding vehicle specific metrics. A GID value (for Nissan Leaf) is the obvious example, but also other indications of battery health. We have SOC%, SOH%, and CAC currently as global metrics.
I think these are adequate, although I think it might also be useful to have "battery energy available" in kW·h, for easier range calculations, (although I see that vehicle modules mostly make their own range calculations, some quite elaborate).
And now to a different thing:
The app supports unlocking and locking of doors. The feature doesn't seem to be implemented for the Nissan Leaf. I have all the CAN-codes needed to make this work. Please let me know if I can contribute.
I'm uncomfortable doing this on my own car, but I'm very happy to have the feature implemented. I think it should be hidden behind a configuration option, so you can choose whether the standard functionality allows the car to be unlocked, but that is a different and not Leaf specific conversation.
I think the protection for this can and should be done globally (irrespective of car model). Bets to keep things as universal as possible, without too many car-specific stuff. Actions like unlock, valet, homelink, etc, should be able to be enabled/disabled at the module level. I will handle the changes for this global protection mechanism. Individual vehicle modules should always implement the functionality if they can. Regards, Mark.
On 2 May 2018, at 3:39 AM, Tom Parker <tom@carrott.org> wrote:
On 02/05/18 01:08, ovms wrote:
Think I found out why I was unable to send mail to this list. (wrong sender address)
My 2016 Leaf still has flapping SOC.
Do you have a 30kWh battery?
I made the following changes in vehicle_nissanleaf.cpp and compiled the code.
case 0x50d: //Changed this from 5bc. 50d is the instrument SOC { uint16_t nl_gids = ((uint16_t) d[0] << 2) | ((d[1] & 0xc0) >> 6); if (nl_gids == 1023) {
break; } uint16_t max_gids = MyConfig.GetParamValueInt("xnl", "maxGids", GEN_1_NEW_CAR_GIDS); float km_per_kwh = MyConfig.GetParamValueFloat("xnl", "kmPerKWh", GEN_1_KM_PER_KWH); float wh_per_gid = MyConfig.GetParamValueFloat("xnl", "whPerGid", GEN_1_WH_PER_GID);
m_gids->SetValue(nl_gids); StandardMetrics.ms_v_bat_soc->SetValue(nl_gids / 4); //Changed this so the value is divided by 4.
SOC now shows exactly the same in the app as the instrument-panel does, and is not flapping. (I know the instrument-panel is different from the true SOC)
Right, so you've confirmed that if module sends a reasonable SOC to the server, the value stops flapping in the app?
My coding capabilities are limited so if someone has a better idea, I can test it on my car.
We can make the SOC calculation configurable so that you can choose to use the instrument panel SOC or an SOC based on the new car battery size. When I took over the leaf support I wanted to use the instrument panel soc too, see Tom Saxton's reply which convincingly dissuaded me wanting to switch: http://lists.openvehicles.com/pipermail/ovmsdev/2016-May/002993.html <http://lists.openvehicles.com/pipermail/ovmsdev/2016-May/002993.html> (this discussion that was before I knew SOC was available so I had some no longer relevant ideas about how to actually implement it).
Now we have the easier to configure v3 platform and more users wanting to use the instrument panel SOC I'll make it configurable. I should be able to get that done tonight.
And now to a different thing:
The app supports unlocking and locking of doors. The feature doesn't seem to be implemented for the Nissan Leaf. I have all the CAN-codes needed to make this work. Please let me know if I can contribute.
I'm uncomfortable doing this on my own car, but I'm very happy to have the feature implemented. I think it should be hidden behind a configuration option, so you can choose whether the standard functionality allows the car to be unlocked, but that is a different and not Leaf specific conversation.
You should be able to implement it quite easily by following the pattern used in CommandStartCharge() in the Leaf code and the Lock and UnLock implementations in the Tesla Roadster and Kia Soul code. Be sure to send the frame to the correct CAN bus. The console logging functions are very useful for confirming what is going on when you're developing.
Thanks for
_______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
On 02/05/18 01:08, ovms wrote:
*case 0x50d**:* //Changed this from 5bc. 50d is the instrument SOC
{
uint16_t nl_gids = ((uint16_t) d[0] << 2) | ((d[1] & 0xc0) >> 6);
if (nl_gids == 1023)
{
break;
}
uint16_t max_gids = MyConfig.GetParamValueInt("xnl", "maxGids", GEN_1_NEW_CAR_GIDS);
float km_per_kwh = MyConfig.GetParamValueFloat("xnl", "kmPerKWh", GEN_1_KM_PER_KWH);
float wh_per_gid = MyConfig.GetParamValueFloat("xnl", "whPerGid", GEN_1_WH_PER_GID);
m_gids->SetValue(nl_gids);
StandardMetrics.ms_v_bat_soc->SetValue(*nl_gids / 4*); //Changed this so the value is divided by 4.
SOC now shows exactly the same in the app as the instrument-panel does, and is not flapping. (I know the instrument-panel is different from the true SOC)
I wasn't able to reproduce this. On my car and in the CAN bus captures I've got, there is no 0x50d frame on the EV bus. Now that I'm back in the house, I see that 0x50d is present on the Car CAN bus, but I don't have any data on what's there. My notes indicate that 0x55b on the EV bus contains the state of charge, and I've implemented this at https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/compare/mas... along with a configuration option to choose how to show the SOC. However 0x55b doesn't work how I expected on my 2016 24kWh car, I see xnl.v.bat.gids 140 xnl.v.bat.soc.instrument 56.3% xnl.v.bat.soc.newcar 49.8221% but the instrument cluster displays 52%. Could it be displaying 56.3 - 5 = 51.3 rounded up to 52? Should I be looking at 0x50d on the car bus to faithfully display what the instrument cluster displays? I'll check tomorrow. Assuming we get this working, should we default to using the instrument cluster state of charge, or the "new car" state of charge? I'm guessing we should go with the instrument cluster value, which is a change from previous behavior on the leaf but is in keeping with Mark's "re-implement the broken calculations by the car".
Are we sure we have the right cables? Remember that on the Nissan Leaf specific cable, the two buses are switched. On the new OBDII generic cables, we have both buses available. Regards, Mark
On 2 May 2018, at 7:55 PM, Tom Parker <tom@carrott.org> wrote:
On 02/05/18 01:08, ovms wrote:
case 0x50d: //Changed this from 5bc. 50d is the instrument SOC { uint16_t nl_gids = ((uint16_t) d[0] << 2) | ((d[1] & 0xc0) >> 6); if (nl_gids == 1023) {
break; } uint16_t max_gids = MyConfig.GetParamValueInt("xnl", "maxGids", GEN_1_NEW_CAR_GIDS); float km_per_kwh = MyConfig.GetParamValueFloat("xnl", "kmPerKWh", GEN_1_KM_PER_KWH); float wh_per_gid = MyConfig.GetParamValueFloat("xnl", "whPerGid", GEN_1_WH_PER_GID);
m_gids->SetValue(nl_gids); StandardMetrics.ms_v_bat_soc->SetValue(nl_gids / 4); //Changed this so the value is divided by 4.
SOC now shows exactly the same in the app as the instrument-panel does, and is not flapping. (I know the instrument-panel is different from the true SOC)
I wasn't able to reproduce this. On my car and in the CAN bus captures I've got, there is no 0x50d frame on the EV bus. Now that I'm back in the house, I see that 0x50d is present on the Car CAN bus, but I don't have any data on what's there.
My notes indicate that 0x55b on the EV bus contains the state of charge, and I've implemented this at https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/compare/mas... <https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/compare/master...carrott:nissan-leaf-soc-instrument?expand=1> along with a configuration option to choose how to show the SOC.
However 0x55b doesn't work how I expected on my 2016 24kWh car, I see
xnl.v.bat.gids 140 xnl.v.bat.soc.instrument 56.3% xnl.v.bat.soc.newcar 49.8221%
but the instrument cluster displays 52%. Could it be displaying 56.3 - 5 = 51.3 rounded up to 52?
Should I be looking at 0x50d on the car bus to faithfully display what the instrument cluster displays? I'll check tomorrow.
Assuming we get this working, should we default to using the instrument cluster state of charge, or the "new car" state of charge? I'm guessing we should go with the instrument cluster value, which is a change from previous behavior on the leaf but is in keeping with Mark's "re-implement the broken calculations by the car". _______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
On Wed, May 02, 2018 at 11:55:19PM +1200, Tom Parker wrote:
On 02/05/18 01:08, ovms wrote:
*case 0x50d**:* //Changed this from 5bc. 50d is the instrument SOC ... SOC now shows exactly the same in the app as the instrument-panel does, and is not flapping. (I know the instrument-panel is different from the true SOC) ... I wasn't able to reproduce this. On my car and in the CAN bus captures I've got, there is no 0x50d frame on the EV bus. Now that I'm back in the house, I see that 0x50d is present on the Car CAN bus, but I don't have any data on what's there.
The top 8 bits of the first byte are the instrument SOC% (on MY2015), but it only updates when the car is on, not when charging. Incidentally, the only other thing I have noted for 0x50d is that there are two bits in d[6] which seem to be related to the charge port latch.
Should I be looking at 0x50d on the car bus to faithfully display what the instrument cluster displays? I'll check tomorrow.
I think it's just a direct repeat of the value in 0x1db, which is more useful because it also updates when charging: case 0x1db: { // sent by the LBC, measured inside the battery box ... StandardMetrics.ms_v_bat_current->SetValue(nl_battery_current / 2.0f); // state of charge percentage, as shown on dash info panel uint8_t soc = d[4] & 0x7f; if(soc != 0x7f) { StandardMetrics.ms_v_bat_soc->SetValue(soc); }
My notes indicate that 0x55b on the EV bus contains the state of charge, and I've implemented this at https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/compare/mas... along with a configuration option to choose how to show the SOC.
However 0x55b doesn't work how I expected on my 2016 24kWh car, I see
xnl.v.bat.gids 140 xnl.v.bat.soc.instrument 56.3% xnl.v.bat.soc.newcar 49.8221%
but the instrument cluster displays 52%. Could it be displaying 56.3 - 5 = 51.3 rounded up to 52?
Yes, the value in the top 10 bits of 0x55b is interesting since it has a few more bits of resolution and updates when charging or driving, but is actually measuring something slightly different from the instrument SOC%---usually not by much, but sometimes a couple of points.
Assuming we get this working, should we default to using the instrument cluster state of charge, or the "new car" state of charge? I'm guessing we should go with the instrument cluster value, which is a change from previous behavior on the leaf but is in keeping with Mark's "re-implement the broken calculations by the car".
That would have my vote too.
Tom, you are right. I used a standard cable, not a Leaf-specific cable. And the 0x50d is on the car-bus. I have now swapped to a Leaf-specific cable, and tried to change the code to look for 0x50d on the car bus, but I apparently did something wrong. No data from the car-bus is received now. My vote (if I have one) is to use the instrument SOC. It´s better from a end-user perspective. -Stein Arne Sordal- On 2018-05-02 15:06, Robin O'Leary <ovmsdev@caederus.org> wrote:
On Wed, May 02, 2018 at 11:55:19PM +1200, Tom Parker wrote: > On 02/05/18 01:08, ovms wrote: > >*case 0x50d**:* //Changed this from 5bc. 50d is the instrument SOC ... > >SOC now shows exactly the same in the app as the instrument-panel does, > >and is not flapping. (I know the instrument-panel is different from the > >true SOC) ... > I wasn't able to reproduce this. On my car and in the CAN bus captures I've > got, there is no 0x50d frame on the EV bus. Now that I'm back in the house, > I see that 0x50d is present on the Car CAN bus, but I don't have any data on > what's there. The top 8 bits of the first byte are the instrument SOC% (on MY2015), but it only updates when the car is on, not when charging. Incidentally, the only other thing I have noted for 0x50d is that there are two bits in d[6] which seem to be related to the charge port latch. > Should I be looking at 0x50d on the car bus to faithfully display what the > instrument cluster displays? I'll check tomorrow. I think it's just a direct repeat of the value in 0x1db, which is more useful because it also updates when charging: case 0x1db: { // sent by the LBC, measured inside the battery box ... StandardMetrics.ms_v_bat_current->SetValue(nl_battery_current / 2.0f); // state of charge percentage, as shown on dash info panel uint8_t soc = d[4] & 0x7f; if(soc != 0x7f) { StandardMetrics.ms_v_bat_soc->SetValue(soc); } > My notes indicate that 0x55b on the EV bus contains the state of charge, and > I've implemented this at https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/compare/mas... > along with a configuration option to choose how to show the SOC. > However 0x55b doesn't work how I expected on my 2016 24kWh car, I see > > xnl.v.bat.gids 140 > xnl.v.bat.soc.instrument 56.3% > xnl.v.bat.soc.newcar 49.8221% > > but the instrument cluster displays 52%. Could it be displaying 56.3 - 5 = > 51.3 rounded up to 52? Yes, the value in the top 10 bits of 0x55b is interesting since it has a few more bits of resolution and updates when charging or driving, but is actually measuring something slightly different from the instrument SOC%---usually not by much, but sometimes a couple of points. > Assuming we get this working, should we default to using the instrument > cluster state of charge, or the "new car" state of charge? I'm guessing we > should go with the instrument cluster value, which is a change from previous > behavior on the leaf but is in keeping with Mark's "re-implement the broken > calculations by the car". That would have my vote too. _______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
On Wed, May 02, 2018 at 03:43:33PM +0200, ovms wrote:
Tom, you are right. I used a standard cable, not a Leaf-specific cable. And the 0x50d is on the car-bus. I have now swapped to a Leaf-specific cable, and tried to change the code to look for 0x50d on the car bus, but I apparently did something wrong. No data from the car-bus is received now.
Can you try to see if adding this code also works for you? After "case 0x1db:" // state of charge percentage, as shown on dash info panel uint8_t soc = d[4] & 0x7f; if(soc != 0x7f) { StandardMetrics.ms_v_bat_soc->SetValue(soc); }
Hi
Can you try to see if adding this code also works for you? >After "case 0x1db:" > // state of charge percentage, as shown on dash info panel > uint8_t soc = d[4] & 0x7f; > if(soc != 0x7f) > { > StandardMetrics.ms_v_bat_soc->SetValue(soc); > }
I tried the change you suggested. SOC is stable now. Nice! -Stein Arne Sordal- On 2018-05-02 15:55, Robin O'Leary <ovmsdev@caederus.org> wrote:
On Wed, May 02, 2018 at 03:43:33PM +0200, ovms wrote: > Tom, you are right. I used a standard cable, not a Leaf-specific cable. And the 0x50d is on the car-bus. > I have now swapped to a Leaf-specific cable, and tried to change the code to look for 0x50d on the car bus, but I apparently did something wrong. No data from the car-bus is received now. Can you try to see if adding this code also works for you? After "case 0x1db:" // state of charge percentage, as shown on dash info panel uint8_t soc = d[4] & 0x7f; if(soc != 0x7f) { StandardMetrics.ms_v_bat_soc->SetValue(soc); } _______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
On Wed, May 02, 2018 at 11:55:19PM +1200, Tom Parker wrote:
My notes indicate that 0x55b on the EV bus contains the state of charge, and I've implemented this at https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/compare/mas... along with a configuration option to choose how to show the SOC.
Comments on this: https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/compare/mas.... A purely stylistic point: Many Leaf CAN packets contain several values, some of which may be valid while others have an "undefined" value (typically all-1s). So I think it's better not to "break" out of the case this: + uint16_t soc10 = ((uint16_t) d[0] << 2) | ((d[1] & 0xc0) >> 6); + if (soc10 == 1023) + { + // ignore invalid data seen during startup + break; + } ... because there may be other valid values in the same packet that won't be processed (there aren't in this case, but when I was adding new metrics, there were situations where this occurred). Better to say: + uint16_t soc10 = ((uint16_t) d[0] << 2) | ((d[1] & 0xc0) >> 6); + if (soc10 != 1023) // ignore invalid data seen during startup + { + float soc = soc10 / 10.0; + m_soc_instrument->SetValue(soc); ... For choosing which to display: + if (MyConfig.GetParamValueBool("xnl", "soc.instrument", false)) { + StandardMetrics.ms_v_bat_soc->SetValue(soc);
Assuming we get this working, should we default to using the instrument cluster state of charge, or the "new car" state of charge? I'm guessing we should go with the instrument cluster value, which is a change from previous behavior on the leaf but is in keeping with Mark's "re-implement the broken calculations by the car".
Instead of creating a new option, could we not use whether or not the existing "newCarAh" option is set? It needs to be set to a correct value in order for the old method to work, and people who don't set it would just get the dash version. I would hazard a guess that most people won't have set newCarAh, so the displayed value will slightly increase for them (depending how degraded their battery is). But it seems likely that most people won't notice the change of behaviour, and if they do, they either won't mind or will be motivated to get it back by setting newCarAh.
On Thu, May 03, 2018 at 10:25:34AM +0100, Robin O'Leary wrote:
Instead of creating a new option, could we not use whether or not the existing "newCarAh" option is set? It needs to be set to a correct value in order for the old method to work, and people who don't set it would just get the dash version.
Sorry, I meant the "maxGids" option, not newCarAh. ...although the same argument applies to ms_v_bat_soh and newCarAh: if newCarAh is set, do it the current way, else use the dash reading.
On 03/05/18 21:25, Robin O'Leary wrote:
On Wed, May 02, 2018 at 11:55:19PM +1200, Tom Parker wrote:
My notes indicate that 0x55b on the EV bus contains the state of charge, and I've implemented this at https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/compare/mas... along with a configuration option to choose how to show the SOC. Comments on this:
https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/compare/mas....
A purely stylistic point: Many Leaf CAN packets contain several values, some of which may be valid while others have an "undefined" value (typically all-1s). So I think it's better not to "break" out of the case this:
+ uint16_t soc10 = ((uint16_t) d[0] << 2) | ((d[1] & 0xc0) >> 6); + if (soc10 == 1023) + { + // ignore invalid data seen during startup + break; + } ...
Good point. I've re-factored the 0x5bc instance of this pattern and pushed to the same branch. I also grabbed the instrument SOC from 0x1db and swapped the default source of the main SOC metric. What do you think? This is displaying the correct instrument cluster soc for me. I also found that I couldn't make the code return to using the instrument SOC return false after populating the config value, setting the config value to any of the not-true values or even removing it altogether didn't make it go back. I haven't looked at what is wrong yet. As a broader concern, I don't really like the giant switch statement with most parsing inline. I ported the v2 code over with as few changes as possible, and in v2 I was a bit paranoid about running out of call stack (the pic processor has a fairly shallow hardware stack, though this fear may be misplaced as the compiler might not have even used it?). In the v3 code we should consider pulling the parsing code into separate functions which are invoked by the big switch. I'm not sure if this would actually be better without trying it, but the big switch certainly feels wrong, not lest because I'm creating blocks inside the case statements and using local variables not visible to the other cases!
Instead of creating a new option, could we not use whether or not the existing "newCarAh" option is set? It needs to be set to a correct value in order for the old method to work, and people who don't set it would just get the dash version.
I think we can discover the battery size from the SOH, CAC, SOC and GIDs metrics, so we can get rid of the newCarAh and newCarGIDS metrics altogether. There may be an explicit signal on the CAN bus which has the new car battery size?
On Thu, May 03, 2018 at 10:51:07PM +1200, Tom Parker wrote:
On 03/05/18 21:25, Robin O'Leary wrote:
On Wed, May 02, 2018 at 11:55:19PM +1200, Tom Parker wrote: As a broader concern, I don't really like the giant switch statement with most parsing inline. I ported the v2 code over with as few changes as possible, and in v2 I was a bit paranoid about running out of call stack (the pic processor has a fairly shallow hardware stack, though this fear may be misplaced as the compiler might not have even used it?). In the v3 code we should consider pulling the parsing code into separate functions which are invoked by the big switch.
Well, there is something to be said for keeping that code path as simple as possible, as it will be run hundreds of times a second---and other vehicles also take the same approach with a big switch(). And to be honest, I don't really see any benefit if we just end up with: ... case 0x54c: parse_54c(d); break; case 0x5bc: parse_5bc(d); break; ... If we are looking to re-factor properly and reduce repeated code, I'd abstract out the common bit-manipulation, test for exceptional value and offset/scale calculations. Many (most?) cases could be covered with these parameters in a table: { address, bit_shift, mask, ignore_value, offset, scale, metric_object } e.g. case 0x54c: /* Ambient temperature. This one has half-degree C resolution, * and seems to stay within a degree or two of the "eyebrow" temp display. * App label: AMBIENT */ if (d[6] != 0xff) { StandardMetrics.ms_v_env_temp->SetValue(d[6] / 2.0 - 40); } Treating d[0..7] as a 64-bit value, d[6] would be >> 8 & 0xff, so this would be expressed in the table as: // address shift mask ignore offset scale metric { 0x54c, 8, 0xff, 0xff, 80, 0.5, StandardMetrics.ms_v_env_temp } This could also handle some more complicated cases with an intermediate metric, e.g. case 0x5bc: { uint16_t nl_gids = ((uint16_t) d[0] << 2) | ((d[1] & 0xc0) >> 6); if (nl_gids == 1023) { // ignore invalid data seen during startup break; } float km_per_kwh = MyConfig.GetParamValueFloat("xnl", "kmPerKWh", GEN_1_KM_PER_KWH); float wh_per_gid = MyConfig.GetParamValueFloat("xnl", "whPerGid", GEN_1_WH_PER_GID); m_gids->SetValue(nl_gids); StandardMetrics.ms_v_bat_soc->SetValue((nl_gids * 100.0) / max_gids); StandardMetrics.ms_v_bat_range_ideal->SetValue((nl_gids * wh_per_gid * km_per_kwh) / 1000); break; } The GID value itself fits the scheme: // address shift mask ignore offset scale metric { 0x5bc, 66, 1023, 1023, 0, 1, m_gids } The SetValue method of m_gids would then be responsible for dealing with the other metrics depending on its value. It seems like this could benefit developers of other vehicles too, so maybe we should have this conversation with a wider audience.
On Thu, May 03, 2018 at 10:51:07PM +1200, Tom Parker wrote:
On 03/05/18 21:25, Robin O'Leary wrote:
On Wed, May 02, 2018 at 11:55:19PM +1200, Tom Parker wrote:
My notes indicate that 0x55b on the EV bus contains the state of charge, and I've implemented this at https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/compare/mas... along with a configuration option to choose how to show the SOC. Comments on this:
https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/compare/mas.... ... I've re-factored the 0x5bc instance of this pattern and pushed to the same branch. I also grabbed the instrument SOC from 0x1db and swapped the default source of the main SOC metric. What do you think?
Looks good.
I think we can discover the battery size from the SOH, CAC, SOC and GIDs metrics, so we can get rid of the newCarAh and newCarGIDS metrics altogether.
There may be an explicit signal on the CAN bus which has the new car battery size?
It seems likely that there is such a thing, though I haven't spotted it. Perhaps that's not surprising as I've done very few comparions between logs from cars with different battery sizes. Do you have any such logs?
On Thu, May 03, 2018 at 10:51:07PM +1200, Tom Parker wrote:
https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/pull/110 ... I also grabbed the instrument SOC from 0x1db and swapped the default source of the main SOC metric. What do you think?
The more I look at it, the more I'm going off the idea of having multiple alternative sources of data and an option of which to promote to the "standard" metric. I think we must try follow Mark's original objective to make the standard SOC metric show what is on the dash; if we want a better metric representing absolute energy (which I do), then there should be a different metric for that (and, indeed, there may be more than one, such as KWh, GIDs, percent of new---though I would suggest only kWh deserves to be a standard metric, while the others would remain vehicle specific). What particularly got me thinking along these lines is that I have been reading some other metrics directly from the Leaf's own instruments, like the distance range and time-to-charge estimates, and again I have the problem of where to put them. There is already code to calculate these values independently, and I am sure some people will again prefer the results of those calulations over the car's "guess-o-meter". But it seems increasingly ridiculous to repeat the same pattern for every one (having two local metrics and a config option for which to display). Surely the job of the vehicle code is just to accurately report what the car says. If we want to calculate better estimates, that doesn't seem to belong in each vehicle module. For example, both the Kia Soul and Twizy have code to apply a temperature correction to the range, but they do it differently. Which way is better? Is that really a vehicle-specific difference, or is it just two independent tries at solving the same tricky problem? I suspect the latter, and that should be done in a more centralized place where better techniques can be shared and more easily fine tuned. Would I be right to think that the original justification for making the SOC behaviour configurable is so the scaled-to-new-SOC can be seen on the phone app? If so, then maybe what we are really missing is a way to customise which metric(s) should be displayed on the phone app (and perhaps other user interfaces like the web status page and dashboard, which may have different capablilites). Looking at a way to add that seems like a better way to go than trying to narrowly solve each display selection issue one by one in the vehicle code.
On 07/05/18 00:43, Robin O'Leary wrote:
What particularly got me thinking along these lines is that I have been reading some other metrics directly from the Leaf's own instruments, like the distance range and time-to-charge estimates, and again I have the problem of where to put them. There is already code to calculate these values independently, and I am sure some people will again prefer the results of those calulations over the car's "guess-o-meter". But it seems increasingly ridiculous to repeat the same pattern for every one (having two local metrics and a config option for which to display). Surely the job of the vehicle code is just to accurately report what the car says.
I agree. (Although in the case of the range calculation, the metrics in v3 are influenced by those in v2 which are heavily influenced by the Tesla Roadster so there are two range metrics displayed in the app: estimated and ideal. I put my calculation into one of them and never got round to fishing the guess-o-meter off the can bus and putting it into the other.)
If we want to calculate better estimates, that doesn't seem to belong in each vehicle module. For example, both the Kia Soul and Twizy have code to apply a temperature correction to the range, but they do it differently. Which way is better? Is that really a vehicle-specific difference, or is it just two independent tries at solving the same tricky problem? I suspect the latter, and that should be done in a more centralized place where better techniques can be shared and more easily fine tuned.
Agreed. There should be a general algorithm that works for all vehicles and if the vehicle is special or to be compatible with some other part of the ecosystem (eg to be identical to some aspect of LeafSpy) it can be overridden by the vehicle module. Other things that spring to mind that might benefit from a shared implementation are Wh/km, charge time remaining, trip & charge session statistics.
Would I be right to think that the original justification for making the SOC behaviour configurable is so the scaled-to-new-SOC can be seen on the phone app? If so, then maybe what we are really missing is a way to customise which metric(s) should be displayed on the phone app (and perhaps other user interfaces like the web status page and dashboard, which may have different capablilites). Looking at a way to add that seems like a better way to go than trying to narrowly solve each display selection issue one by one in the vehicle code.
Yes, it's working around limitations in the v2 server architecture. While it's relatively easy to add new metrics to the v2 protocol, adding a vehicle specific metric isn't so easy, as it introduces a dependency between the v2 server code and the vehicle module. The v3 server looks like it solves this problem as it can send all the metrics regardless of whether they are contributed by a vehicle module or are part of the core metrics list. Are we better to wait for the v3 server/client ecosystem and then move the configuration from the car module to the client application, or should we add a leaf specific metric to the v2 protocol now and put the configuration in the client?
Are we better to wait for the v3 server/client ecosystem and then move the configuration from the car module to the client application, or should we add a leaf specific metric to the v2 protocol now and put the configuration in the client?
I wouldn’t bother with v2. Just create the v3 metric as vehicle-specific. Over the coming months, the bluetooth GATT client will come, and v3 MQTT support, as well as App support for v3 MQTT and GATT. I’m not intending to work any more on the v2 App. Regards, Mark.
On 7 May 2018, at 8:03 PM, Tom Parker <tom@carrott.org> wrote:
On 07/05/18 00:43, Robin O'Leary wrote:
What particularly got me thinking along these lines is that I have been reading some other metrics directly from the Leaf's own instruments, like the distance range and time-to-charge estimates, and again I have the problem of where to put them. There is already code to calculate these values independently, and I am sure some people will again prefer the results of those calulations over the car's "guess-o-meter". But it seems increasingly ridiculous to repeat the same pattern for every one (having two local metrics and a config option for which to display). Surely the job of the vehicle code is just to accurately report what the car says.
I agree. (Although in the case of the range calculation, the metrics in v3 are influenced by those in v2 which are heavily influenced by the Tesla Roadster so there are two range metrics displayed in the app: estimated and ideal. I put my calculation into one of them and never got round to fishing the guess-o-meter off the can bus and putting it into the other.)
If we want to calculate better estimates, that doesn't seem to belong in each vehicle module. For example, both the Kia Soul and Twizy have code to apply a temperature correction to the range, but they do it differently. Which way is better? Is that really a vehicle-specific difference, or is it just two independent tries at solving the same tricky problem? I suspect the latter, and that should be done in a more centralized place where better techniques can be shared and more easily fine tuned.
Agreed. There should be a general algorithm that works for all vehicles and if the vehicle is special or to be compatible with some other part of the ecosystem (eg to be identical to some aspect of LeafSpy) it can be overridden by the vehicle module. Other things that spring to mind that might benefit from a shared implementation are Wh/km, charge time remaining, trip & charge session statistics.
Would I be right to think that the original justification for making the SOC behaviour configurable is so the scaled-to-new-SOC can be seen on the phone app? If so, then maybe what we are really missing is a way to customise which metric(s) should be displayed on the phone app (and perhaps other user interfaces like the web status page and dashboard, which may have different capablilites). Looking at a way to add that seems like a better way to go than trying to narrowly solve each display selection issue one by one in the vehicle code.
Yes, it's working around limitations in the v2 server architecture. While it's relatively easy to add new metrics to the v2 protocol, adding a vehicle specific metric isn't so easy, as it introduces a dependency between the v2 server code and the vehicle module. The v3 server looks like it solves this problem as it can send all the metrics regardless of whether they are contributed by a vehicle module or are part of the core metrics list.
Are we better to wait for the v3 server/client ecosystem and then move the configuration from the car module to the client application, or should we add a leaf specific metric to the v2 protocol now and put the configuration in the client?
_______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
participants (5)
-
Greg D. -
Mark Webb-Johnson -
ovms -
Robin O'Leary -
Tom Parker