<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body style="overflow-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;">Sorry,<div><br></div><div>I reviewed the code before this, and that changes things somewhat:</div><div><br></div><blockquote style="margin: 0 0 0 40px; border: none; padding: 0px;"><div><div><font face="Andale Mono">void OvmsVehicleFactory::SetVehicle(const char* type)</font></div><div><font face="Andale Mono"> {</font></div><div><font face="Andale Mono"> if (m_currentvehicle)</font></div><div><font face="Andale Mono"> {</font></div><div><font face="Andale Mono"> m_currentvehicle->m_ready = false;</font></div><div><font face="Andale Mono"> delete m_currentvehicle;</font></div><div><font face="Andale Mono"> m_currentvehicle = NULL;</font></div><div><font face="Andale Mono"> m_currentvehicletype.clear();</font></div><div><font face="Andale Mono"> MyEvents.SignalEvent("vehicle.type.cleared", NULL);</font></div><div><font face="Andale Mono"> }</font></div><div><font face="Andale Mono"> m_currentvehicle = NewVehicle(type);</font></div><div><font face="Andale Mono"> if (m_currentvehicle)</font></div><div><font face="Andale Mono"> {</font></div><div><font face="Andale Mono"> m_currentvehicle->m_ready = true;</font></div><div><font face="Andale Mono"> }</font></div><div><font face="Andale Mono"> m_currentvehicletype = std::string(type);</font></div><div><font face="Andale Mono"> StandardMetrics.ms_v_type->SetValue(m_currentvehicle ? type : "");</font></div><div><font face="Andale Mono"> MyEvents.SignalEvent("vehicle.type.set", (void*)type, strlen(type)+1);</font></div><div><font face="Andale Mono"> }</font></div></div></blockquote><div><br></div><div>So I think the change could be:</div><div><br></div><blockquote style="margin: 0 0 0 40px; border: none; padding: 0px;"><div style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><font face="Andale Mono">...</font></div><div style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><font face="Andale Mono">m_currentvehicle = NewVehicle(type);</font></div><div style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><font face="Andale Mono">if (m_currentvehicle)</font></div><div style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><font face="Andale Mono"> {</font></div><div style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><font face="Andale Mono"> m_currentvehicletype = std::string(type);</font></div><div style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><font face="Andale Mono"> m_currentvehicle->m_ready = true;</font></div><div style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="font-family: "Andale Mono";"> StandardMetrics.ms_v_type->SetValue(type);</span></div><div style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="font-family: "Andale Mono";"> MyEvents.SignalEvent("vehicle.type.set", (void*)type, strlen(type)+1);</span></div><div></div><div style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><font face="Andale Mono"> }</font></div><div style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><font face="Andale Mono">}</font></div></blockquote><div><div><br></div><div>Regards, Mark</div><div><br><blockquote type="cite"><div>On 27 Nov 2023, at 3:24 PM, Mark Webb-Johnson <mark@webb-johnson.net> wrote:</div><br class="Apple-interchange-newline"><div><meta http-equiv="content-type" content="text/html; charset=utf-8"><div style="overflow-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;"><meta http-equiv="content-type" content="text/html; charset=utf-8"><div style="overflow-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;">Michael,<div><br></div><div>I’m not too bothered about it at the moment, but if you would to put in a PR for treating the “if (m_currentvehicle)” as set ready, set type, signal, and then an else for set type “” only, that would be acceptable to me.</div><div><br></div><div>Regards, Mark.<br><div><br><blockquote type="cite"><div>On 27 Nov 2023, at 3:14 PM, Michael Geddes <frog@bunyip.wheelycreek.net> wrote:</div><br class="Apple-interchange-newline"><div><div dir="ltr">So, would you be better if (in the case of null vehicle) m_currentVehicletype was also set to "" and we didn't fire the signal event? <div><br></div><div> I was doing some other work in the area and noticed this. <br><div><br></div><div>//.ichael</div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 27 Nov 2023 at 07:37, Mark Webb-Johnson <<a href="mailto:mark@webb-johnson.net">mark@webb-johnson.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>Michael,<div><br></div><div>From the condition in the <font face="monospace">v_type->SetValue, the type is only set if NewVehicle(type) is not NULL.</font></div><div><font face="monospace"><br></font></div><div><font face="monospace">So the question is whether we should raise the signal or not? I guess that is debatable, and probably better not to (as otherwise it is being raised with type as the parameter, but the type is not actually being set as the vehicle type).<br></font><div><br></div><div>Regards, Mark</div><div><br><blockquote type="cite"><div>On 26 Nov 2023, at 9:26 AM, Michael Geddes <<a href="mailto:frog@bunyip.wheelycreek.net" target="_blank">frog@bunyip.wheelycreek.net</a>> wrote:</div><br><div><div dir="ltr"><div>This code is from OvmsVehicleFactor::SetVehicle<br></div><div><br></div><font face="monospace"> m_currentvehicle = NewVehicle(type);<br> if (m_currentvehicle)<br> {<br> m_currentvehicle->m_ready = true;<br> }<br> m_currentvehicletype = std::string(type);<br> StandardMetrics.ms_v_type->SetValue(m_currentvehicle ? type : "");<br> MyEvents.SignalEvent("vehicle.type.set", (void*)type, strlen(type)+1);<br></font><div><br></div><div>My question is about what happens when NewVehicle() returns NULL.</div><div><br></div><div>Should m_currecntvehicletype, v.type and the SignalEvent all be set as blank? Any particular reason why only the v.type is set as blank?</div><div><br></div><div>//.ichael</div></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" target="_blank">http://lists.openvehicles.com/mailman/listinfo/ovmsdev</a><br></div></blockquote></div><br></div></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>
_______________________________________________<br>OvmsDev mailing list<br>OvmsDev@lists.openvehicles.com<br>http://lists.openvehicles.com/mailman/listinfo/ovmsdev<br></div></blockquote></div><br></div></div></div>_______________________________________________<br>OvmsDev mailing list<br>OvmsDev@lists.openvehicles.com<br>http://lists.openvehicles.com/mailman/listinfo/ovmsdev<br></div></blockquote></div><br></div></body></html>