This code is from OvmsVehicleFactor::SetVehicle m_currentvehicle = NewVehicle(type); if (m_currentvehicle) { m_currentvehicle->m_ready = true; } m_currentvehicletype = std::string(type); StandardMetrics.ms_v_type->SetValue(m_currentvehicle ? type : ""); MyEvents.SignalEvent("vehicle.type.set", (void*)type, strlen(type)+1); My question is about what happens when NewVehicle() returns NULL. 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? //.ichael
Michael, From the condition in the v_type->SetValue, the type is only set if NewVehicle(type) is not NULL. 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). Regards, Mark
On 26 Nov 2023, at 9:26 AM, Michael Geddes <frog@bunyip.wheelycreek.net> wrote:
This code is from OvmsVehicleFactor::SetVehicle
m_currentvehicle = NewVehicle(type); if (m_currentvehicle) { m_currentvehicle->m_ready = true; } m_currentvehicletype = std::string(type); StandardMetrics.ms_v_type->SetValue(m_currentvehicle ? type : ""); MyEvents.SignalEvent("vehicle.type.set", (void*)type, strlen(type)+1);
My question is about what happens when NewVehicle() returns NULL.
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?
//.ichael _______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
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? I was doing some other work in the area and noticed this. //.ichael On Mon, 27 Nov 2023 at 07:37, Mark Webb-Johnson <mark@webb-johnson.net> wrote:
Michael,
From the condition in the v_type->SetValue, the type is only set if NewVehicle(type) is not NULL.
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).
Regards, Mark
On 26 Nov 2023, at 9:26 AM, Michael Geddes <frog@bunyip.wheelycreek.net> wrote:
This code is from OvmsVehicleFactor::SetVehicle
m_currentvehicle = NewVehicle(type); if (m_currentvehicle) { m_currentvehicle->m_ready = true; } m_currentvehicletype = std::string(type); StandardMetrics.ms_v_type->SetValue(m_currentvehicle ? type : ""); MyEvents.SignalEvent("vehicle.type.set", (void*)type, strlen(type)+1);
My question is about what happens when NewVehicle() returns NULL.
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?
//.ichael _______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
_______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
Michael, 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. Regards, Mark.
On 27 Nov 2023, at 3:14 PM, Michael Geddes <frog@bunyip.wheelycreek.net> wrote:
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?
I was doing some other work in the area and noticed this.
//.ichael
On Mon, 27 Nov 2023 at 07:37, Mark Webb-Johnson <mark@webb-johnson.net <mailto:mark@webb-johnson.net>> wrote:
Michael,
From the condition in the v_type->SetValue, the type is only set if NewVehicle(type) is not NULL.
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).
Regards, Mark
On 26 Nov 2023, at 9:26 AM, Michael Geddes <frog@bunyip.wheelycreek.net <mailto:frog@bunyip.wheelycreek.net>> wrote:
This code is from OvmsVehicleFactor::SetVehicle
m_currentvehicle = NewVehicle(type); if (m_currentvehicle) { m_currentvehicle->m_ready = true; } m_currentvehicletype = std::string(type); StandardMetrics.ms_v_type->SetValue(m_currentvehicle ? type : ""); MyEvents.SignalEvent("vehicle.type.set", (void*)type, strlen(type)+1);
My question is about what happens when NewVehicle() returns NULL.
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?
//.ichael _______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com <mailto:OvmsDev@lists.openvehicles.com> http://lists.openvehicles.com/mailman/listinfo/ovmsdev
_______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com <mailto:OvmsDev@lists.openvehicles.com> http://lists.openvehicles.com/mailman/listinfo/ovmsdev
OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
Sorry, I reviewed the code before this, and that changes things somewhat: void OvmsVehicleFactory::SetVehicle(const char* type) { if (m_currentvehicle) { m_currentvehicle->m_ready = false; delete m_currentvehicle; m_currentvehicle = NULL; m_currentvehicletype.clear(); MyEvents.SignalEvent("vehicle.type.cleared", NULL); } m_currentvehicle = NewVehicle(type); if (m_currentvehicle) { m_currentvehicle->m_ready = true; } m_currentvehicletype = std::string(type); StandardMetrics.ms_v_type->SetValue(m_currentvehicle ? type : ""); MyEvents.SignalEvent("vehicle.type.set", (void*)type, strlen(type)+1); } So I think the change could be: ... m_currentvehicle = NewVehicle(type); if (m_currentvehicle) { m_currentvehicletype = std::string(type); m_currentvehicle->m_ready = true; StandardMetrics.ms_v_type->SetValue(type); MyEvents.SignalEvent("vehicle.type.set", (void*)type, strlen(type)+1); } } Regards, Mark
On 27 Nov 2023, at 3:24 PM, Mark Webb-Johnson <mark@webb-johnson.net> wrote:
Michael,
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.
Regards, Mark.
On 27 Nov 2023, at 3:14 PM, Michael Geddes <frog@bunyip.wheelycreek.net> wrote:
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?
I was doing some other work in the area and noticed this.
//.ichael
On Mon, 27 Nov 2023 at 07:37, Mark Webb-Johnson <mark@webb-johnson.net <mailto:mark@webb-johnson.net>> wrote:
Michael,
From the condition in the v_type->SetValue, the type is only set if NewVehicle(type) is not NULL.
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).
Regards, Mark
On 26 Nov 2023, at 9:26 AM, Michael Geddes <frog@bunyip.wheelycreek.net <mailto:frog@bunyip.wheelycreek.net>> wrote:
This code is from OvmsVehicleFactor::SetVehicle
m_currentvehicle = NewVehicle(type); if (m_currentvehicle) { m_currentvehicle->m_ready = true; } m_currentvehicletype = std::string(type); StandardMetrics.ms_v_type->SetValue(m_currentvehicle ? type : ""); MyEvents.SignalEvent("vehicle.type.set", (void*)type, strlen(type)+1);
My question is about what happens when NewVehicle() returns NULL.
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?
//.ichael _______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com <mailto:OvmsDev@lists.openvehicles.com> http://lists.openvehicles.com/mailman/listinfo/ovmsdev
_______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com <mailto:OvmsDev@lists.openvehicles.com> http://lists.openvehicles.com/mailman/listinfo/ovmsdev
OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
_______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
I was also thinking adding this: else { StandardMetrics.ms_v_type->Clear(); } On Mon, 27 Nov 2023 at 15:25, Mark Webb-Johnson <mark@webb-johnson.net> wrote:
Michael,
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.
Regards, Mark.
On 27 Nov 2023, at 3:14 PM, Michael Geddes <frog@bunyip.wheelycreek.net> wrote:
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?
I was doing some other work in the area and noticed this.
//.ichael
On Mon, 27 Nov 2023 at 07:37, Mark Webb-Johnson <mark@webb-johnson.net> wrote:
Michael,
From the condition in the v_type->SetValue, the type is only set if NewVehicle(type) is not NULL.
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).
Regards, Mark
On 26 Nov 2023, at 9:26 AM, Michael Geddes <frog@bunyip.wheelycreek.net> wrote:
This code is from OvmsVehicleFactor::SetVehicle
m_currentvehicle = NewVehicle(type); if (m_currentvehicle) { m_currentvehicle->m_ready = true; } m_currentvehicletype = std::string(type); StandardMetrics.ms_v_type->SetValue(m_currentvehicle ? type : ""); MyEvents.SignalEvent("vehicle.type.set", (void*)type, strlen(type)+1);
My question is about what happens when NewVehicle() returns NULL.
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?
//.ichael _______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
_______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
_______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
_______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
participants (2)
-
Mark Webb-Johnson -
Michael Geddes