<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">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>