openvehicles / Open-Vehicle-Monitoring-System-3

Open Vehicle Monitoring System - Version 3
http:///www.openvehicles.com/
Other
599 stars 228 forks source link

Nissan Leaf: partially sending V2 "S" messages once per second while charging #684

Open dexterbg opened 2 years ago

dexterbg commented 2 years ago

Ref: https://www.openvehicles.com/node/2942

I've found multiple sections like this is Peter's log:

2022-01-18 08:15:55 C rx msg S 98.0,M,220,10.00,charging,standard,129,88,27,64,0,0,3,1,0,0,0,0,67.68,1440,0,0,0,-1,0,0,0,0,0,89,0,-1.17,391.50,85,2.20,53.39,3.00,-4.0
2022-01-18 08:15:56 C rx msg S 98.0,M,220,10.45,charging,standard,129,88,26,65,0,0,3,1,0,0,0,0,67.68,1440,0,0,0,-1,0,0,0,0,0,89,0,-1.76,391.00,85,2.30,53.39,4.50,-6.0
2022-01-18 08:15:57 C rx msg S 98.0,M,220,11.36,charging,standard,129,88,26,66,0,0,3,1,0,0,0,0,67.68,1440,0,0,0,-1,0,0,0,0,0,89,0,-1.17,391.00,85,2.50,53.39,3.00,-4.0
2022-01-18 08:15:58 C rx msg S 98.0,M,220,11.36,charging,standard,129,88,26,67,0,0,3,1,0,0,0,0,67.68,1440,0,0,0,-1,0,0,0,0,0,89,0,-1.37,391.00,85,2.50,53.39,3.50,-4.6
2022-01-18 08:16:01 C rx msg S 98.0,M,220,11.36,charging,standard,129,88,26,70,0,0,3,1,0,0,0,0,67.68,1440,0,0,0,-1,0,0,0,0,0,89,0,-1.37,391.00,85,2.50,53.39,3.50,-4.6

My suspect are frequent minor value changes of one of the metrics defined as important by the V2 server client, main suspect being v.c.climit / ms_v_charge_climit. That seems to be updated from multiple CAN sources and the updating is done as a division of power by voltage with result set at maximum float resolution.

So if the power reading jitters by +/- 1 Watt, every such event will cause an immediate "S" update.

@dalathegreat / @mjkapkan / @drc38 -- can someone please have a look at this?

I suggest rounding/truncing readings triggering updates to an appropriate precision, see TRUNCPREC and ROUNDPREC helper macros in ovms_utils.h for this.

Thanks, Michael

mjkapkan commented 2 years ago

Just so I understand, these messages are supposed to be sent when the charging power changes, but they are being sent even when the power level is not changing?Or it's just that granularity we have in NL code is too high?

mjkapkan commented 2 years ago

Also I can confirm I see them being sent each second as well. With the value going between 14.5455A and 15A.

dexterbg commented 2 years ago

It's the granularity of the change. The metric is a float metric, so can e.g. have 26.0001A. This will be sent as 26A via the "S" message, as that currently converts the float to int (may be changed itf, but then to something like 1/10A precision).

If you now change the metric value to 26.0002, "S" will be sent again as the V2 code only sees the metric has changed and assumes it needs to transmit an update, but the message field will still read "26".

Rounding or truncating precision to 1/10A would be sufficient, as the charge current limit doesn't need high precision.

dexterbg commented 2 years ago

14.5455 vs 15A … rounding to 1/10 wouldn't help if you see jitter of that size. Maybe you need to round to 1A then.

dexterbg commented 2 years ago

After all, it's the charge current limit, the maximum charger output current, that's normally a fixed property of the charging station, possibly in combination with a configured fixed limit -- it's not meant to change that often.

drc38 commented 2 years ago

So changing to AsInt() when setting the metric should fix it?

dexterbg commented 2 years ago

AsInt() is a getter, you could wrap the value in round(…) or ROUNDPREC(…, 0). But I saw multiple code places for different CAN IDs where the metric is set, so you should also check if some of them possibly fight over the metric.

dalathegreat commented 2 years ago

Uncommented one of the sources (in 0x380) Test build available here: https://dalasevrepair.fi/wp-content/OVMS/ovms3.bin WIP PR up here: https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/pull/685

dalathegreat commented 2 years ago

So with the log file I got from @dexterbg , that doesn't seem to solve the problem. For me, issue does not appear when using a 3.3kW (16A charger), only when charging on a 1.6kW (8A) charger.

I am a bit stumped, what could cause the spam now that we don't set the ms_v_charge_climit in 0x380?

dexterbg commented 2 years ago

In the log the climit mostly fluctuates around 23A (22-24), then near the end rises to 27A, seemingly correlating to the decline of the charge current.

This must be coming from one of these:

dalathegreat commented 2 years ago

@dexterbg The first climit is set in the 2013-2017 0x390 message

The second climit is set int the 2011-2012 LEAF 0x5BF message

So they should not compete for the value.

EDIT: I will make a test build without any climit writing so that we can find out if this is the reason. EDIT2: Test build without climit: https://dalasevrepair.fi/wp-content/OVMS/ovms3.bin

dexterbg commented 2 years ago

Daniel, Peter tried your latest build (without climit) today, and had no more 'S' spam.

Regards, Michael

dalathegreat commented 2 years ago

Thanks Michael, I updated the PR with another step forward: https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/pull/685

drc38 commented 2 years ago

Hi Michael, as the presentation layer precision can differ from the underlying metric, is it possible to provide the IsModifiedAndClear an optional parameter (based on presentation precision eg int) that it uses to test for a change rather than metric precision eg float?

Although for climit is probably doesn't matter I can see for other metrics you might want the metric precision to differ from what's presented in the app.

dexterbg commented 2 years ago

Derek,

the modification bitset is a property of the metric. To make that aware of the listener's presentation precisions, the metric would need to keep a copy of it's value for each listener and run through all listeners on each value change.

It's the task of the listener to apply additional change detection filters as needed. I've suggested adding an option for a more tolerant filter for the server v2 listener in issue #683 to reduce overall data usage. That may well result in a generally applicable presentation precision aware mechanism.

The general standard way is to truncate/round float/double values to a reasonable precision before assigning them to a float metric.

The charge current limit was initially defined as an integer metric, alined with the integer precision in the V2 protocol. We initially had a range of voltage/power/current metrics defined as integers, which then turned out should have been floats, so we changed all of these metrics to float to have the precision option. The climit metric could have been excluded from that rework, as a charger normally has fixed levels (e.g. 5A / 8A / 10A / 13A / 16A), but cars may use the metric to transport their own idea of a charge current limit -- as I did on the Twizy and you do on the Leaf.

So if you use it for the car's charge current limit and derive it from a highly volatile power reading, you need to cut down the precision appropriately, e.g. using the macros meant for this (see above), and possibly also add a smoothing filter.

Here is an example for a combined filter + precision truncation:

https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/blob/7ac23a856068e03f453391a910b4bfba878e8b98/vehicle/OVMS.V3/components/vehicle/vehicle.cpp#L1578-L1581

Regards, Michael