<html>
<head>
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p><br>
</p>
<br>
<div class="moz-cite-prefix">On 20/04/18 03:39, Robin O'Leary wrote:
</div>
<blockquote type="cite" cite="mid:20180419153903.GB17346@mail.ro.nu">
<pre wrap="">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.</pre>
</blockquote>
<br>
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.<br>
<br>
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.<br>
<br>
I'd be pleased to replace these klunky bits and pieces with more
direct detection of the car's state.<br>
<br>
<blockquote type="cite" cite="mid:20180419153903.GB17346@mail.ro.nu">
<pre wrap="">What is the process for contributing changes? I am currently doing
my local work on the master branch cloned from github.</pre>
</blockquote>
<br>
Looking at your pull request I have a couple of comments.<br>
<br>
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).<br>
<br>
Your comments on <span class="blob-code-inner"><span class="pl-c1">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 </span></span><span class="blob-code-inner"><span
class="pl-c1"><span class="blob-code-inner">d[<span
class="pl-c1">0</span>] == <span class="pl-c1">0x40 to</span></span>
</span></span><span class="blob-code-inner"><span class="pl-c1"><span
class="blob-code-inner">(d[<span class="pl-c1">0</span>]>><span
class="pl-c1">6</span>) == <span class="pl-c1">1</span>,
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?</span></span></span><br>
<span class="blob-code-inner"><span class="pl-c1"><span
class="blob-code-inner"></span></span></span><br>
<span class="blob-code-inner"><span class="pl-c1"><span
class="blob-code-inner">I'm already polling the SOH from the
BMS with more precision than is available on </span></span></span><span
class="blob-code-inner"><span class="pl-c1"><span
class="blob-code-inner"><span class="blob-code-inner"><span
class="pl-c1">0x5b3</span></span></span>, 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 </span></span><span
class="blob-code-inner"><span class="pl-c1"><span
class="blob-code-inner"><span class="pl-c1"><span
class="blob-code-inner"><span class="blob-code-inner"><span
class="pl-c1">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</span></span></span></span></span></span></span><span
class="blob-code-inner"><span class="pl-c1"><span
class="blob-code-inner"><span class="pl-c1"><span
class="blob-code-inner"><span class="blob-code-inner"><span
class="pl-c1"><span class="blob-code-inner"><span
class="pl-c1"><span class="blob-code-inner"><span
class="pl-c1"><span class="blob-code-inner"><span
class="blob-code-inner"><span
class="pl-c1"> (CAC /</span></span></span></span></span></span></span>
SOH_5b3 * 100) > 70. That would remove the need to
configure the correct Ah to get the SOH calculation to
work.</span></span></span></span></span></span></span><br>
<span class="blob-code-inner"><span class="pl-c1"><span
class="blob-code-inner"><span class="pl-c1"><span
class="blob-code-inner"><span class="blob-code-inner"><span
class="pl-c1"></span></span></span></span></span></span></span><br>
<span class="blob-code-inner"><span class="pl-c1"><span
class="blob-code-inner"><span class="pl-c1"><span
class="blob-code-inner"><span class="blob-code-inner"><span
class="pl-c1">The SOC in </span></span></span></span></span></span></span><span
class="blob-code-inner"><span class="pl-c1"><span
class="blob-code-inner"><span class="pl-c1"><span
class="blob-code-inner"><span class="blob-code-inner"><span
class="pl-c1"><span class="pl-c1">0x1db</span> 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</span></span></span></span></span></span></span><span
class="blob-code-inner"><span class="pl-c1"><span
class="blob-code-inner"><span class="pl-c1"><span
class="blob-code-inner"><span class="blob-code-inner"><span
class="pl-c1"><span class="blob-code-inner"><span
class="pl-c1"><span class="blob-code-inner"><span
class="pl-c1"><span class="blob-code-inner"><span
class="blob-code-inner"><span
class="pl-c1"> to "fully charged is
100%"</span></span></span></span></span></span></span>:
<a class="moz-txt-link-freetext" href="http://lists.openvehicles.com/pipermail/ovmsdev/2016-May/002993.html">http://lists.openvehicles.com/pipermail/ovmsdev/2016-May/002993.html</a>
(this discussion that was before I knew SOC was
available so I had some no longer relevant ideas about
how to actually implement it).</span></span></span></span></span></span></span><br>
<br>
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.<br>
<br>
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!<br>
<br>
<br>
</body>
</html>