Re: https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/issues/120 I've pushed an update to our mongoose clone that should solve the spurious crashes in mbuf_insert(). After the next git pull, you will also need to do a git submodule update. Please test for your setups and report if the crash still occurs or any new issues come up. Regards, Michael -- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
Michael, This seems a very neat solution, at a fundamental level, to the problem. I just worry if there are other parts of mongoose (other than buffers) that are going to hit this multi-threaded issue. I think, for safety, we need to make sure as much as possible is done inside the mongoose task. Please let us know if it solves the problem you are seeing. Regards, Mark.
On 24 May 2018, at 11:30 PM, Michael Balzer <dexter@expeedo.de> wrote:
Re: https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/issues/120
I've pushed an update to our mongoose clone that should solve the spurious crashes in mbuf_insert().
After the next git pull, you will also need to do a git submodule update.
Please test for your setups and report if the crash still occurs or any new issues come up.
Regards, Michael
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
Am 25.05.2018 um 04:01 schrieb Mark Webb-Johnson:
Michael,
This seems a very neat solution, at a fundamental level, to the problem. I just worry if there are other parts of mongoose (other than buffers) that are going to hit this multi-threaded issue. I think, for safety, we need to make sure as much as possible is done inside the mongoose task.
Yes, that's an important note for all developers: *this does not make mongoose thread safe*. This only makes the connection buffer access thread safe so mg_send() can be called from other tasks, e.g. from event or metric listeners. This can be done if sends are atomic on the protocol level (as done in the v2 server by Transmit()). A clean "mongoose style" solution for this would be to queue all transmissions and only send from the mongoose event handler or a mongoose broadcast handler. All other concurrent accesses to mongoose connections still must be avoided. Even API calls that seem to only do read accesses (e.g. mg_conn_addr_to_str) can have hidden side effects if called from another context.
Please let us know if it solves the problem you are seeing.
This was happening mostly on the Twizy because it currently does the most custom telemetry. Looks very good so far. I rolled out the build yesterday evening, so I will have some more data on this over the weekend. Regards, Michael
Regards, Mark.
On 24 May 2018, at 11:30 PM, Michael Balzer <dexter@expeedo.de> wrote:
Re: https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/issues/120
I've pushed an update to our mongoose clone that should solve the spurious crashes in mbuf_insert().
After the next git pull, you will also need to do a git submodule update.
Please test for your setups and report if the crash still occurs or any new issues come up.
Regards, Michael
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ 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 Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
Tom Parker: Any chance you can implement door control to the Leaf? It´s working great with this changes (the door parts): vehicle_nissanleaf.cpp: switch (command) { case ENABLE_CLIMATE_CONTROL: ESP_LOGI(TAG, "Enable Climate Control"); data[0] = 0x4e; data[1] = 0x08; data[2] = 0x12; data[3] = 0x00; break; case DISABLE_CLIMATE_CONTROL: ESP_LOGI(TAG, "Disable Climate Control"); data[0] = 0x56; data[1] = 0x00; data[2] = 0x01; data[3] = 0x00; break; case START_CHARGING: ESP_LOGI(TAG, "Start Charging"); data[0] = 0x66; data[1] = 0x08; data[2] = 0x12; data[3] = 0x00; break; case UNLOCK_DOORS: ESP_LOGI(TAG, "Unlook Doors"); data[0] = 0x11; data[1] = 0x00; data[2] = 0x00; data[3] = 0x00; break; case LOCK_DOORS: ESP_LOGI(TAG, "Look Doors"); data[0] = 0x60; data[1] = 0x80; data[2] = 0x00; data[3] = 0x00; break; default: OvmsVehicle::vehicle_command_t OvmsVehicleNissanLeaf::CommandClimateControl(bool climatecontrolon) { ESP_LOGI(TAG, "CommandClimateControl"); return RemoteCommandHandler(climatecontrolon ? ENABLE_CLIMATE_CONTROL : DISABLE_CLIMATE_CONTROL); } OvmsVehicle::vehicle_command_t OvmsVehicleNissanLeaf::CommandLock(const char* pin) { return RemoteCommandHandler(LOCK_DOORS); } OvmsVehicle::vehicle_command_t OvmsVehicleNissanLeaf::CommandUnlock(const char* pin) { return RemoteCommandHandler(UNLOCK_DOORS); } vehicle_nissanleaf.h: typedef enum { ENABLE_CLIMATE_CONTROL, DISABLE_CLIMATE_CONTROL, START_CHARGING, UNLOCK_DOORS, LOCK_DOORS } RemoteCommand; public: void IncomingFrameCan1(CAN_frame_t* p_frame); void IncomingFrameCan2(CAN_frame_t* p_frame); vehicle_command_t CommandHomelink(int button); vehicle_command_t CommandClimateControl(bool enable); virtual vehicle_command_t CommandLock(const char* pin); virtual vehicle_command_t CommandUnlock(const char* pin); void RemoteCommandTimer(); Kind regards, Stein Arne Sordal
On Fri, May 25, 2018 at 10:32:05AM +0200, Stein Arne Sordal wrote:
Any chance you can implement door control to the Leaf?
I made a github branch for it: https://github.com/caederus-ovms/Open-Vehicle-Monitoring-System-3/tree/leaf-...
It´s working great with this changes (the door parts):
Unfortunately, I just tried on MY2015, and these commands seemed to have no effect. But maybe that isn't much of a surprise: my older model doesn't claim to have remote lock/unlock, and the architecture is a lot different (using frames with only 1 byte instead of 4, and on a different bus). Did you have to unplug your TCU? Mine is still plugged in; maybe I need to try it unplugged.
Yes, my TCU is unplugged. It´s a 2016 model. I´ll try to borrow a friends older Leaf and see if it works. -Stein Arne-
On 25 May 2018, at 15:44, Robin O'Leary <ovmsdev@caederus.org> wrote:
On Fri, May 25, 2018 at 10:32:05AM +0200, Stein Arne Sordal wrote:
Any chance you can implement door control to the Leaf?
I made a github branch for it:
https://github.com/caederus-ovms/Open-Vehicle-Monitoring-System-3/tree/leaf-...
It´s working great with this changes (the door parts):
Unfortunately, I just tried on MY2015, and these commands seemed to have no effect. But maybe that isn't much of a surprise: my older model doesn't claim to have remote lock/unlock, and the architecture is a lot different (using frames with only 1 byte instead of 4, and on a different bus).
Did you have to unplug your TCU? Mine is still plugged in; maybe I need to try it unplugged. _______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
On 26/05/18 01:44, Robin O'Leary wrote:
Unfortunately, I just tried on MY2015, and these commands seemed to have no effect. But maybe that isn't much of a surprise: my older model doesn't claim to have remote lock/unlock, and the architecture is a lot different (using frames with only 1 byte instead of 4, and on a different bus).
Given the remote door lock Stein has uses the same sequence the TCU uses for remote climate control, I wouldn't be too surprised if it doesn't work on cars that didn't have that feature from the beginning.
Did you have to unplug your TCU? Mine is still plugged in; maybe I need to try it unplugged.
The other remote commands don't work properly when the TCU is plugged in, so it's possible this is your problem. I have both a 2016 and 2012 Leaf, but unfortunately I swapped cars with a friend for the weekend so I'll have to wait for it to come back with my OVMS v3 before testing. The other thing to do is to record the can bus and see what happens when you send the unlock sequence on a car that supports Nissan Connect unlock, and also when you unlock the doors with the key remote. Possibly it can be implemented on cars that don't support remote unlock in Nissan Connect by mimicking the key remote, or some combination of the Nissan Connect wake-up message and the key remote unlock messages. The "lock doors above a speed" feature may also be informative, but you'd have to pick it out of the messages sent while the car is moving.
On 26/05/18 15:58, Tom Parker wrote:
On 26/05/18 01:44, Robin O'Leary wrote:
Unfortunately, I just tried on MY2015, and these commands seemed to have no effect. But maybe that isn't much of a surprise: my older model doesn't claim to have remote lock/unlock, and the architecture is a lot different (using frames with only 1 byte instead of 4, and on a different bus).
Given the remote door lock Stein has uses the same sequence the TCU uses for remote climate control, I wouldn't be too surprised if it doesn't work on cars that didn't have that feature from the beginning.
Did you have to unplug your TCU? Mine is still plugged in; maybe I need to try it unplugged.
The other remote commands don't work properly when the TCU is plugged in, so it's possible this is your problem. I have both a 2016 and 2012 Leaf, but unfortunately I swapped cars with a friend for the weekend so I'll have to wait for it to come back with my OVMS v3 before testing.
Stein's code from your branch works on my 2016 car with the TCU plugged in (I still have the TCU plugged in because the hands free microphone doesn't work when it's simply unplugged). I couldn't work out how to make the android app send an unlock command though, it would only let me lock the doors. Does the car have to signal it is locked before the app will invite you to lock it? I'll test on a 2012 car later today, but I'm pessimistic it will work on my car -- it needs to be woken up with a hardware signal rather than a CAN frame. The wakeup process changed to entirely CAN based in the 2013 model year and I've had reports some 2012 cars don't need the hardware wakeup. I haven't set up my OVMS v3 to make the necessary signal, so it won't be able to wake the car. On my 2016 model with the TCU connected, it locks the car multiple times. When I originally wrote the remote climate control and charge control code, I copied what the TCU sends, which is hte same message repeated for a second or so. Perhaps this is unnecessary on the 2016 cars, or on the lock signal? Stein do you have can bus captures of Nissan Connect doing the lock and unlock?
On 29 May 2018, at 21:43, Tom Parker <tom@carrott.org> wrote:
On 26/05/18 15:58, Tom Parker wrote:
On 26/05/18 01:44, Robin O'Leary wrote:
Unfortunately, I just tried on MY2015, and these commands seemed to have no effect. But maybe that isn't much of a surprise: my older model doesn't claim to have remote lock/unlock, and the architecture is a lot different (using frames with only 1 byte instead of 4, and on a different bus).
Given the remote door lock Stein has uses the same sequence the TCU uses for remote climate control, I wouldn't be too surprised if it doesn't work on cars that didn't have that feature from the beginning.
Did you have to unplug your TCU? Mine is still plugged in; maybe I need to try it unplugged.
The other remote commands don't work properly when the TCU is plugged in, so it's possible this is your problem. I have both a 2016 and 2012 Leaf, but unfortunately I swapped cars with a friend for the weekend so I'll have to wait for it to come back with my OVMS v3 before testing.
Stein's code from your branch works on my 2016 car with the TCU plugged in (I still have the TCU plugged in because the hands free microphone doesn't work when it's simply unplugged). I couldn't work out how to make the android app send an unlock command though, it would only let me lock the doors. Does the car have to signal it is locked before the app will invite you to lock it?
I'll test on a 2012 car later today, but I'm pessimistic it will work on my car -- it needs to be woken up with a hardware signal rather than a CAN frame. The wakeup process changed to entirely CAN based in the 2013 model year and I've had reports some 2012 cars don't need the hardware wakeup. I haven't set up my OVMS v3 to make the necessary signal, so it won't be able to wake the car.
On my 2016 model with the TCU connected, it locks the car multiple times. When I originally wrote the remote climate control and charge control code, I copied what the TCU sends, which is hte same message repeated for a second or so. Perhaps this is unnecessary on the 2016 cars, or on the lock signal?
Stein do you have can bus captures of Nissan Connect doing the lock and unlock?
Nissan Connect does not support remote lock/unlock of doors on earlier than 2018 models.
_______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
I missed adding the mutex lock to mbuf_append() and mbuf_trim(). Fix is pushed, remember to also do "git submodule update". Regards, Michael Am 25.05.2018 um 09:29 schrieb Michael Balzer:
Am 25.05.2018 um 04:01 schrieb Mark Webb-Johnson:
Michael,
This seems a very neat solution, at a fundamental level, to the problem. I just worry if there are other parts of mongoose (other than buffers) that are going to hit this multi-threaded issue. I think, for safety, we need to make sure as much as possible is done inside the mongoose task.
Yes, that's an important note for all developers: *this does not make mongoose thread safe*.
This only makes the connection buffer access thread safe so mg_send() can be called from other tasks, e.g. from event or metric listeners. This can be done if sends are atomic on the protocol level (as done in the v2 server by Transmit()). A clean "mongoose style" solution for this would be to queue all transmissions and only send from the mongoose event handler or a mongoose broadcast handler.
All other concurrent accesses to mongoose connections still must be avoided. Even API calls that seem to only do read accesses (e.g. mg_conn_addr_to_str) can have hidden side effects if called from another context.
Please let us know if it solves the problem you are seeing.
This was happening mostly on the Twizy because it currently does the most custom telemetry.
Looks very good so far. I rolled out the build yesterday evening, so I will have some more data on this over the weekend.
Regards, Michael
Regards, Mark.
On 24 May 2018, at 11:30 PM, Michael Balzer <dexter@expeedo.de> wrote:
Re: https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/issues/120
I've pushed an update to our mongoose clone that should solve the spurious crashes in mbuf_insert().
After the next git pull, you will also need to do a git submodule update.
Please test for your setups and report if the crash still occurs or any new issues come up.
Regards, Michael
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ 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 Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
participants (5)
-
Mark Webb-Johnson -
Michael Balzer -
Robin O'Leary -
Stein Arne Sordal -
Tom Parker