Add ability to report short-lived low 12v conditions

[chris8086]

I found it very useful to be able to track the minimum 12 volt level as well as the average. The minimum is what's needed to diagnose communication DTCs where the root cause is insufficient voltage. This PR adds a metric v.b.12v.voltage.min and two ways of using it.

For all cars it can generate warning entries in the log when the car is awake and voltage is below the new config parameter 12v.low_warning_awake (default 11.5, although personally I prefer 12.0).

For those with Server v3, the metric is available to be periodically reported. To make this useful, the value is reset every time it's reported so you can track the minimum over time. It would be possible to add it to Server v2 as well, but I didn't go there as I suspect the protocol means clients need to be aware of the additional field.

I tried to make this fit the style of the surrounding code. Happy to make any changes desired, for example if it's felt there should be a Web GUI field for 12v.low_warning_awake.

[dexterbg]

Chris,

that's a useful idea.

Remarks & change requests regarding your implementation:

First of all, you need to be aware why we do the ADC value smoothing (multisampling), and what that means to your method of taking the minimum: we saw a lot of misreadings and noise on single reads. That's an ESP32 hardware issue, Espressif recommend adding a capacitor (which we did) plus doing multisampling:

https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/peripherals/adc_calibration.html#minimize-noise

So if you now rely on single reads for the minimum, that will lead to false alerts, and especially not be a reliable measurement and not reflect the actual minimum voltage.

The smoothing is done over 4 reads (= 4 seconds currently, as HousekeepingUpdate12V() is currently called from the per second ticker). If that's not sufficient to detect the Leaf-killing drops, you need to rework the complete 12V reading to be called from a separate higher frequency timer (e.g. running with a 250 ms period, so you get four measurements per second). Taking multiple samples at once doesn't help here, as the noise is influenced by the momentary ESP32 hardware state (e.g. current Wifi activity).

This will also fix a side issue of your voltage value: you should round the value (max 2 decimals) before assigning it to a metric. Keeping the maximum float precision with this reading is misleading, as the actual precision of the ADC is much lower. Rounding the value also avoids unnecessary metric updates.

When moving the 12V read to a timer context, be aware you must not use the ESP_LOG functions within the timer callback, as they can block. Only do the value updates in the timer, delegate all higher level processing to the per second ticker handler. That might as well (or even better) be the one for the vehicle module, as we do the other 12V processing there already.

Second: metrics listeners (like server v3) must not alter metrics (except those originating at the channel, like client counts). Metrics are shared among all listeners. If a metric listener alters or clears a metric on update, the other listeners down the line don't get the correct value.

Metrics changes also trigger their transmission. That means by clearing the metric after every transmission, you force a retransmission for each new read. That will lead to a lot of additional traffic especially for V3/MQTT users, which is quite expensive in many countries (e.g. USA).

So that metric clearing needs to be removed from the V3 server, and it must not be added elsewhere – the concept of clearing the metric "after the report" is wrong. If you need to keep track of a "has been reported" state, that can be done by adding some state variable at the/each component doing the/a report.

To prevent unnecessary metric transmissions, all you need to do is taking care of only updating the metric when a new minimum is detected. I.e. only do the SetValue() call if the new value is lower than the current value. To make that work you need to reset the value to the current value when a new usage cycle begins. That would naturally be done after a 12V charging & calmdown phase, i.e. basically when the new 12V reference voltage is also taken (again, see vehicle module).

Also, this generally calls for an event: if a state detection or change is so important it needs to be reported in any way, that means it's important enough to enable other components and applications to react on it as well, so that detection should trigger an event. A default event listener can then also take care of issuing any higher level user notifications.

Side note: if you add the state change detection to the vehicle ticker, follow the vehicle class convention of having a standard notification method, that can be suppressed or overridden. See the 12V monitoring for pattern templates, e.g.:

MyEvents.SignalEvent("vehicle.alert.12v.on", NULL);
if (m_autonotifications) Notify12vCritical();

The Notify… method can do whatever type of notification is needed / configured.

Third: your change to OvmsMetricFloat::Clear() doesn't make sense and is probably based on a misunderstanding:

// OvmsMetric::Clear() sets value to "", so make it a float again
m_value = 0.0;

The SetValue(string) call is virtual, so mapped to OvmsMetricFloat::SetValue(string), which converts "" to float. Also, a float metric's m_value is always a float, and we don't use NaN here. The SetValue(0) may be superfluous, but that's a change for another PR, as that pattern applies to all metrics subclasses.

Regarding V2 protocol inclusion:

V2 clients don't need to be aware of new fields, as the protocol is designed to allow adding fields at the end.

I'd also say this metric is clearly a candidate for V2 inclusion, as it's an indication of a severe vehicle failure condition on at least the Leaf, possibly others as well. Also, most OVMS users stick to V2 to avoid the MQTT traffic overhead. So please add this metric to the "D" message and update the documentation accordingly:

https://docs.openvehicles.com/en/latest/protocol_v2/messages.html#car-environment-message-0x44-d

Regarding the web UI:

Currently the config variable only controls logging. The log is a developer tool. The UI should focus on user level features & tools. So unless you add some push notification along with the log entry, this config should not be added to the UI.

But you might consider adding a display for the new metric, e.g. to the 12V monitor config card or to the live status card. That's done by simply adding a div like this (see status card's 12V display):

<div class="metric number" data-metric="v.b.12v.voltage.min" data-prec="1">
  <span class="value">?</span><span class="unit">V</span>
</div>

OK… this has become quite long. Apologies for underdocumenting the basic concepts of the OVMS framework, we really need to put some effort into that.

Don't hesitate to ask if you need more info.

Regards,
Michael

[chris8086]

Michael, thank you for your detailed consideration!

ADC measurements

In which version did the capacitor get added to the hardware? The ADC noise comparison chart shows that single samples with a capacitor vary by +/- 0.3% compared to the smoothed value. My view is that's sufficient accuracy. The Grafana chart I sent to the list, and subsequent data, doesn't lead me to think that I'm seeing spurious minimum values. I'm thinking that if the hardware has the capacitor, then the code should be ok with monitoring individual ADC reads.

Happy to add rounding to 2dp.

Reporting minimum value over time

Happy to remove resetting of the min from the metric listener.

The question becomes the definition of the min. I think you are suggesting that the min normally only decreases, with increases occurring rarely (i.e. after charging and calmdown). That delivers less benefit. The current code allows the user to observe a yellow line (min) that normally tracks slightly under the green (smoothed) but every now and then it drops a long way down. From there it might stay there for a bit and then return back up near the green. The frequency of these events can be observed, and correlated to what was going on at the time.

If we change this to a minimum-over-many-hours then the smaller events are hidden, as are the durations of these events. You're only left with the ones that are a new extreme. There's still value in that of course, but it's not as good as a minimum-over-polling-interval.

If you agree with trying to report min over a shorter duration, then the question is how. I think it makes sense to measure min over m_updatetime_*, e.g. m_updatetime_awake. OvmsServerV3::Ticker1() has some code that chooses the current appropriate updatetime. Of course we could define a new config that sets a fixed duration for min tracking, but I feel it ought to vary according to the "awakeness" of the car.

OvmsMetricFloat

I added this because I saw a bug where the code read the value of the float metric and it wasn't a float. Adding this solved it. I'll have to take a deeper look at the virtual handling here. Quite possibly my misunderstanding.

V2

Ok, good to know it's easily extendable.

Web UI

12V monitor is a good idea. Will have a think.

[dexterbg]

ADC measurements

0.3% would be acceptable, but the capacitor wasn't added until main board v3.3. Also, we saw higher noise levels than those shown in the Espressif graph. Espressif show noise levels of a plain ESP32, in our environment the modem may have some additional influence on the noise.

Reporting minimum value over time

I see your point, I thought it's sufficient to keep the worst (lowest) level seen.

To get a graph that also allows counting those dips, you can do a peak detection and only update the min when that triggers, or you can apply an asymmetric smoothing filter to the readings (similar to a VU meter's peak & hold algorithm).

A peak detection has already been implemented by @frogonwheels in commit f1b06e3 for the smoothed 12V signal. You could even simply hook into the "dip" event generated there, if you could use the smoothed value.

An asymmetric filter takes in one direction (here: drops) with a very fast (or no) smoothing, and applies a slower smoothing to the other direction. That way it immediately adapts to a new peak, then slowly falls back to the baseline. By tuning the smoothing factor, you can adjust the fallback speed. It shouldn't be too slow, so a close followup peak with a lower amplitude won't get hidden.

A peak holding average_util_t can easily be derived by changing add() to test the sign of the delta and apply the smoothing only to one direction.