On 20/04/18 03:39, Robin O'Leary wrote:

Since then, I got a development environment set up with remarkably
little trouble, and I've been adding some new metrics for the Nissan Leaf:
    * ms_v_bat_soh from 0x5b3[1]
    * ms_v_charge_temp from 0x54f[0] (this is actually inside temp)
    * ms_v_door_fl etc. from 0x60d[0]
    * ms_v_env_gear from 0x421[0]
    * ms_v_env_handbrake from 0x5c5[0] (was faked by vehicle_nissanleaf_car_on)
    * ms_v_env_headlights from 0x60d[0,1]
    * ms_v_env_on from 0x60d[1] (was faked by vehicle_nissanleaf_car_on)
    ...but continue to fake ms_v_env_awake by CAN activity of 0x284
    * ms_v_env_locked from 0x60d[2]
    * ms_v_inv_temp from 0x510[7]
    * ms_v_tpms_fl_p etc. from 0x385[2..5]
These all seem to be working nicely (at least, on my UK MY2015).

I have a few questions:

I would like to record the temperature inside the car, but I couldn't
see an appropriate metric (I've stuck it on ms_v_charge_temp for now).
Is there a better place for it?

What is the intent of ms_v_env_on?  It was previously set true when
certain CAN traffic was present and false after a period of inactivity.
I now set it to true when the car is in state "ready to drive".

A similar question goes for ms_v_env_awake.  I've left this doing the
CAN activity detection, which means it changes for minor things like
detecting a key-fob.

I originally wrote this using a 2012 car and the OVMS v2 on the EV can bus, and at the time I didn't know about the VCM status information that is available on the EV bus, so I picked a frame that's only sent when the car is pretty awake, 0x284 is I believe a relayed message from the ABS system. When I learnt about the VCM status information, and also the gear shift status, I didn't bother to update the simple "is the ABS on" logic that I originally used as it works well.

The v3 code is an almost verbatim port of v2. I've extended it a little bit here and there but the structure is the same.

I'd be pleased to replace these klunky bits and pieces with more direct detection of the car's state.

What is the process for contributing changes?  I am currently doing
my local work on the master branch cloned from github.

Looking at your pull request I have a couple of comments.

We're using Mark's "Whitesmiths" coding style which does take some getting used to. Are coding styles discussed in the developer manual anywhere? Sorry if they're not. Can you add braces to the single line if statements and spacing around operators. (I'm not anymore using an IDE that enforces this style so I sometimes fall back to my normal style).

Your comments on 0x5c0 aren't quite right. This /is/ the battery temperature, I've verified that by modifying the value using a man in the middle and watching the battery temperature gauge on the instrument cluster go up and down. It also lines up with the temperatures that you can poll out of the BMS. You changed the flag check from d[0] == 0x40 to (d[0]>>6) == 1, I'm just staring at CAN bus captures and on the car's I've seen, the 0x40 test works but I'm happy to change if there is even a trivial reason to change, do you have any insight into which test is better?

I'm already polling the SOH from the BMS with more precision than is available on 0x5b3, but that calculation needs to know the "new car" Ah battery size to convert from the current Ah to the SOH. Can you store the 0x5b3 SOH in a different metric (or a class variable?) to avoid clobbering the existing SOH value. It would be great if you used the 5b3 SOH to discover the correct battery size. Something like the following ought to be true for a 30kWh car and false for a 24kWh (CAC / SOH_5b3 * 100) > 70. That would remove the need to configure the correct Ah to get the SOH calculation to work.

The SOC in 0x1db is a fundamentally different value than that which is currently calculated by the OVMS. The 1db value is 100% when fully charged, while what we have now is a percentage of a new car's battery and the value at full charge goes down over time as the battery degrades. My 2012 car shows 73% at full charge, while my 2016 car shows 96%. Can you remove this change from your pull request and we can discuss whether to make it configurable which SOC calculation to use. See Tom Saxton's reply which convincingly dissuaded me wanting to switch to "fully charged is 100%": 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).

Did you mean to remove the odometer code? Can you put it back and see if it works with an odometer in miles, I've only been able to test with my km odometers.

Thank you for pulling all this new data out of the car CAN bus, I haven't spent much time on that bus since the v2 hardware couldn't talk to it!