<div dir="ltr"><div dir="ltr">I can live with those astyle settings, so I'll include them.<div><br></div><div>I've been using vim: tabstop=2 shiftwidth=2 expandtabs (which is the same as --indent=spaces=2 afaict) - so that's a relief. Mixed tabs and spaces are a pitb.. </div><div><br></div><div>While I am used to delphi and so have been using the style that puts the brace at the indent level l'm happy to go with the one you have chosen. </div><div>I really dislike that style with the brace indented. While I still like much about C++, especially the template engine, I do miss delphi enums, sets and a few other features!<br></div><div><br></div><div>I'll download astyle and make it nice before I push up. I've been making lots of little commits as I'm going, but will probably just squash it into one initial.</div><div><br></div><div>I'm wondering what I should do about my instrumented function calls! For example - All my functions look like this at the moment (with XARM() and XDISARM)</div><div><br></div><div><div style="color:rgb(212,212,212);background-color:rgb(30,30,30);font-family:Consolas,"Courier New",monospace;font-size:14px;line-height:19px;white-space:pre"><div><span style="color:rgb(86,156,214)">bool</span> <span style="color:rgb(78,201,176)">OvmsBatteryMon</span>::<span style="color:rgb(220,220,170)">checkStateChange</span>()</div><div>{</div><div> <span style="color:rgb(86,156,214)">XARM</span>(<span style="color:rgb(206,145,120)">"OvmsBatteryMon::checkStateChange"</span>);</div><div> <span style="color:rgb(197,134,192)">if</span> (!<span style="color:rgb(156,220,254)">m_dirty</span>)</div><div> {</div><div> <span style="color:rgb(86,156,214)">XDISARM</span>;</div><div> <span style="color:rgb(197,134,192)">return</span> <span style="color:rgb(86,156,214)">false</span>;</div><div> }</div><div> <span style="color:rgb(78,201,176)">OvmsBatteryState</span> <span style="color:rgb(156,220,254)">newState</span> = <span style="color:rgb(220,220,170)">calc_state</span>();</div><div> <span style="color:rgb(86,156,214)">bool</span> <span style="color:rgb(156,220,254)">res</span> = <span style="color:rgb(156,220,254)">newState</span> != <span style="color:rgb(156,220,254)">m_lastState</span>;</div><div> <span style="color:rgb(156,220,254)">m_lastState</span> = <span style="color:rgb(156,220,254)">newState</span>;</div><div> <span style="color:rgb(156,220,254)">m_dirty</span> = <span style="color:rgb(86,156,214)">false</span>;</div><div> <span style="color:rgb(86,156,214)">XDISARM</span>;</div><div> <span style="color:rgb(197,134,192)">return</span> <span style="color:rgb(156,220,254)">res</span>;</div><div>}</div><br></div></div><div><br></div><div>These are Macros so they would process to blank for the release. The idea is that if the object created by XARM() is not disarmed, it will cause a log to happen. (ie if there's an unexpected exception). Should I just leave them in? </div><div><br></div><div>//.ichael</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 3 Oct 2022 at 14:26, Michael Balzer <<a href="mailto:dexter@expeedo.de">dexter@expeedo.de</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
Thanks, Michael.<br>
<br>
> The main problem was the diversity of tab style.<br>
<br>
Yes… I see you even tried to fix indentation style where it seems to
have been mixed unintentionally by the module developers ^_^<br>
<br>
Btw, I've begun including EditorConfigs and Artistic Style configs
for newer modules, e.g.:<br>
-
<a href="https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/blob/master/vehicle/OVMS.V3/components/zip/.editorconfig" target="_blank">https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/blob/master/vehicle/OVMS.V3/components/zip/.editorconfig</a><br>
-
<a href="https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/blob/master/vehicle/OVMS.V3/components/vehicle_vweup/src/.astylerc" target="_blank">https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/blob/master/vehicle/OVMS.V3/components/vehicle_vweup/src/.astylerc</a><br>
<br>
Regards,<br>
Michael<br>
<br>
<br>
<div>Am 03.10.22 um 04:07 schrieb Michael
Geddes:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Pull request created.<br>
<div><br>
</div>
<div>I hope it's ok. </div>
<div>The main problem was the diversity of tab style.</div>
<div><br>
</div>
<div>//.ichael</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Mon, 3 Oct 2022 at 09:23,
Michael Geddes <<a href="mailto:frog@bunyip.wheelycreek.net" target="_blank">frog@bunyip.wheelycreek.net</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr">
<div dir="ltr">Not 8 issues. 8 Files that I found issues
in, each with a bunch of them - so around 50 all up.</div>
<div dir="ltr"><br>
<div>I did a vimgrep for lines starting with something
like 'const char* ident =' ish... </div>
<div><br>
</div>
<div>Yeah, I agree, checking for IsDefined() is a lot less
cludgy, so have done that (yay for vim macros). Also the
existing code didn't cope with -ve values!</div>
<div><br>
</div>
<div>I guess I better fork the project now and submit a
pull request!! :) </div>
<div>//.ichael</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Sun, 2 Oct 2022 at
22:01, Michael Balzer <<a href="mailto:dexter@expeedo.de" target="_blank">dexter@expeedo.de</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div> Michael,<br>
<br>
oh, the dreadful c_str() trap once again… nice find.<br>
<br>
The deallocated heap buffer *can* have been
reallocated easily, we're running on a preemptive OS
and on two CPU cores.<br>
<br>
IOW, all of these need to be fixed. Sure there are
only 8 of them?<br>
<br>
On your is_empty_metric() utility: may be I miss
something, but why don't we simply use the metric's
IsDefined() method?<br>
<br>
Regards,<br>
Michael<br>
<br>
<br>
<div>Am 02.10.22 um 13:37 schrieb Michael Geddes:<br>
</div>
<blockquote type="cite">
<div dir="auto">
<div dir="ltr">Hi all,
<div><br>
</div>
<div>I noticed some behavior on the <b>tpms</b>
command I'd copied from the Kia/Kona code,
which sent me down a small rabbit hole.</div>
<div>What I was seeing was that the temperature
was repeated instead of seeing the pressure
and temperature.</div>
<div><br>
</div>
<div>Even in vehicle.cpp, we can see code like
this:</div>
<div dir="auto"><br>
</div>
<div> <font face="monospace">const char*
range_est =
StdMetrics.ms_v_bat_range_est->AsUnitString("-",
rangeUnit, 0).c_str();<br>
if (*range_est != '-')<br>
writer->printf("Est. range: %s\n",
range_est);</font></div>
<div dir="auto"><font face="monospace"><br>
</font></div>
<div>My C++ is a bit rusty, however I believe
that the 'const char*' won't hold the
temporary std::string object returned by
'AsUnitString'. To do that, I believe we need
to assign to a reference (which I know will
work). The problem is that the original code
will mostly work (especially in cases like the
above), as the deallocated heap memory won't
have been reallocated.</div>
<div><br>
<font face="monospace"> const
std::string& range_est =
StdMetrics.ms_v_bat_range_est->AsUnitString("-",
rangeUnit, 0);<br>
if (!is_empty_metric(range_est))<br>
writer->printf("Est. range: %s\n",
range_est.c_str());<br>
</font></div>
<div><br>
</div>
<div>Where I've defined in vehicle.h the
following:</div>
<div><font face="monospace">inline bool
is_empty_metric(const std::string
&measure)<br>
{<br>
return (measure == "") || (measure[0] ==
'-');<br>
}<br>
</font><br>
</div>
<div>Will the above work functionally? Would it
be better to be: </div>
<div dir="auto"><br>
</div>
<div dir="auto">return (measure =="") ||
(measure == "-") </div>
<div dir="auto"><br>
</div>
<div>This occurs in 8 files (outside my new
Ioniq 5 files) that I have noted. </div>
<div dir="auto"><br>
</div>
<div>I would make a separate commit for this and
do a pull request for it, </div>
<div><br>
</div>
<div>//.ichael</div>
<div><br>
</div>
</div>
</div>
<br>
<fieldset></fieldset>
<pre>_______________________________________________
OvmsDev mailing list
<a href="mailto:OvmsDev@lists.openvehicles.com" target="_blank">OvmsDev@lists.openvehicles.com</a>
<a href="http://lists.openvehicles.com/mailman/listinfo/ovmsdev" target="_blank">http://lists.openvehicles.com/mailman/listinfo/ovmsdev</a>
</pre>
</blockquote>
<br>
<pre cols="72">--
Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal
Fon 02333 / 833 5735 * Handy 0176 / 206 989 26</pre>
</div>
_______________________________________________<br>
OvmsDev mailing list<br>
<a href="mailto:OvmsDev@lists.openvehicles.com" target="_blank">OvmsDev@lists.openvehicles.com</a><br>
<a href="http://lists.openvehicles.com/mailman/listinfo/ovmsdev" rel="noreferrer" target="_blank">http://lists.openvehicles.com/mailman/listinfo/ovmsdev</a><br>
</blockquote>
</div>
</div>
</blockquote>
</div>
<br>
<fieldset></fieldset>
<pre>_______________________________________________
OvmsDev mailing list
<a href="mailto:OvmsDev@lists.openvehicles.com" target="_blank">OvmsDev@lists.openvehicles.com</a>
<a href="http://lists.openvehicles.com/mailman/listinfo/ovmsdev" target="_blank">http://lists.openvehicles.com/mailman/listinfo/ovmsdev</a>
</pre>
</blockquote>
<br>
<pre cols="72">--
Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal
Fon 02333 / 833 5735 * Handy 0176 / 206 989 26</pre>
</div>
_______________________________________________<br>
OvmsDev mailing list<br>
<a href="mailto:OvmsDev@lists.openvehicles.com" target="_blank">OvmsDev@lists.openvehicles.com</a><br>
<a href="http://lists.openvehicles.com/mailman/listinfo/ovmsdev" rel="noreferrer" target="_blank">http://lists.openvehicles.com/mailman/listinfo/ovmsdev</a><br>
</blockquote></div></div>