Allow listing of errors via CANopen SDO #37
Conversation
This exports the list of errors and time at which they occurred as two CANopen SDO arrays. The sub- index is used as an index into the list of errors. Any invalid indices return zero. Clients can use the "lasterr" parameter database spot-value which is present in all libopeninv projects to map the error number to a string. Tests: - Verify with updated OpenInverter CAN Tool that errors can be listed. - Initiate an new error after a period of time and ensure the list tallies with the elapsed wall-clock time. - Check CANopen SDO decodes as expected in Wireshark
|
As a client it would be useful to know the duration of a "tick" when converting from ticks to wall-clock time. Unfortunately not all projects use 10ms tick durations. The ZombieVerter VCU uses a 1 second tick duration. I plan on determining the tick duration from the units field of the "uptime" variable. This is a useful convention to have in projects IMHO as it allows you to spot unscheduled reboots. An alternative thought would be to have a new special sub-index on 0x5000 which could return the tick duration in milliseconds. Thoughts? |
|
Very good implementation, always ment to add it. Thanks! |
* Allow listing of errors via CANopen SDO (#37) This exports the list of errors and time at which they occurred as two CANopen SDO arrays. The sub- index is used as an index into the list of errors. Any invalid indices return zero. Clients can use the "lasterr" parameter database spot-value which is present in all libopeninv projects to map the error number to a string. Tests: - Verify with updated OpenInverter CAN Tool that errors can be listed. - Initiate an new error after a period of time and ensure the list tallies with the elapsed wall-clock time. - Check CANopen SDO decodes as expected in Wireshark * Allow concurrent sending of CAN frames - partial fix for issue #40 Stm32Can::Send() can be called from multiple contexts in a typical libopeninv application (main loop, CAN RX interrupt or timer interrupt(s)). To avoid CAN TX frames being lost we should disable interrupts while we manipulate the TX mailbox and queue. Tests: - On a ZombieVerterVCU experiencing issue #40 repeatedly request clean parameter databases with: while true; do oic cache clean && oic -n 3 -t 300 read opmode; done - Verify databases are downloaded correctly every time - Verify ox100 and 0x300 CAN frames sent to VW SBox continue to be sent * Fix hard realtime performance and have reliable CAN send This changes uses the capability of ARM Cortex-M3 CPUs to raise the interrupt priority mask so that low priority interrupts do not interrupt CAN sending. High priority interrupts form PWM, overcurrent and emergency stop in applications like stm32-sine will be processed unimpeded. High priority interrupts, obviously, cannot send CAN messages. The change requires: - CAN_MAX_IRQ_PRIORITY to be defined in hwdefs.h - The priority of all interrupts that can send CAN frames be 1 or lower (bigger number, lower priority) Tests: - Patch code into Stm32-vcu and stm32-sine - Torture test database download with: while true; do oic cache clean && oic read opmode; done - On stm32-sine set canperiod to 10ms - Ensure stm32-sine is in Run. Check jitter on exciter output with a DSO with infinite perrsistence. The new code shows no jitter. The older code shows a jitter of around 5.2 uSec.
* Here is an untested implementation of timeout mechanism for PutChar to handle #39 Call TriggerTimeout() in a fixed period interval * Fix CAN SDO print timeout (#42) The class needs to be properly initialised as the TriggerTimeout() method will likely be called before any incoming SDO frames. On the VCU I observed unitialised member variables being filled with garbage from the stack painting. The previous TriggerTimeout() logic causes timeouts even when no timeout period has elapsed. This corrupts the parameter database download. Overshooting and setting to zero if required seems to work reliably. I don't understand. Tests: - Repeatedly start a db download, abort and immediate retry. This should fail and it does. - Repeatedly start a db download, abort and wait a few seconds to retry. This should succeed and it does. - Use callingFrequency values of 97 and 103 to force over and undershoots. Verify the timeout behaviour works as expected. * Cleanup CanMap, CanSdo and Stm32Can declarations (#43) Make single parameter constructors explicit. Use "override" on all overridden virtual methods. Remove unused timestamp from CanMap. Remove unused GetFlashAddress from Stm32Can. Tests: - Build as part of VCU project * Allow concurrent sending of CAN frames - partial fix for issue #40 (#41) * Allow listing of errors via CANopen SDO (#37) This exports the list of errors and time at which they occurred as two CANopen SDO arrays. The sub- index is used as an index into the list of errors. Any invalid indices return zero. Clients can use the "lasterr" parameter database spot-value which is present in all libopeninv projects to map the error number to a string. Tests: - Verify with updated OpenInverter CAN Tool that errors can be listed. - Initiate an new error after a period of time and ensure the list tallies with the elapsed wall-clock time. - Check CANopen SDO decodes as expected in Wireshark * Allow concurrent sending of CAN frames - partial fix for issue #40 Stm32Can::Send() can be called from multiple contexts in a typical libopeninv application (main loop, CAN RX interrupt or timer interrupt(s)). To avoid CAN TX frames being lost we should disable interrupts while we manipulate the TX mailbox and queue. Tests: - On a ZombieVerterVCU experiencing issue #40 repeatedly request clean parameter databases with: while true; do oic cache clean && oic -n 3 -t 300 read opmode; done - Verify databases are downloaded correctly every time - Verify ox100 and 0x300 CAN frames sent to VW SBox continue to be sent * Fix hard realtime performance and have reliable CAN send This changes uses the capability of ARM Cortex-M3 CPUs to raise the interrupt priority mask so that low priority interrupts do not interrupt CAN sending. High priority interrupts form PWM, overcurrent and emergency stop in applications like stm32-sine will be processed unimpeded. High priority interrupts, obviously, cannot send CAN messages. The change requires: - CAN_MAX_IRQ_PRIORITY to be defined in hwdefs.h - The priority of all interrupts that can send CAN frames be 1 or lower (bigger number, lower priority) Tests: - Patch code into Stm32-vcu and stm32-sine - Torture test database download with: while true; do oic cache clean && oic read opmode; done - On stm32-sine set canperiod to 10ms - Ensure stm32-sine is in Run. Check jitter on exciter output with a DSO with infinite perrsistence. The new code shows no jitter. The older code shows a jitter of around 5.2 uSec. * Don't force projects to define interrupt priorities to fix CAN send (#44) Realistically only stm32-sine needs to keep PWM timer interrupts running while CAN frames are being queued for sending. To ease updating of libopeninv just disable interrupts for all projects unless CAN_MAX_IRQ_PRIORITY is defined. Tests: - Verify correct operation on an otherwise unmodified Stm32-vcu - Verify correct operation with stm32-sine where CAN_MAX_IRQ_PRIORITY is defined --------- Co-authored-by: David J. Fiddes <35607151+davefiddes@users.noreply.github.com>
|
Just wondering whether the implementation and its dependency to errormessage.h would live a happier life in sdocommands.cpp ? sdocommands is a bit more "userspacy" than cansdo |
|
I struggle to see the difference between At a code level I can see that decoupling CanSdo from the various actions would make sense. Refactoring ProcessSDO() to use a dispatch table would make it a lot clearer. Doing this without loosing efficiency would be a bit of work. Something something constexpr, consteval, etc... |
|
Ah yes, was looking at it purely at code level, didn't consider the subtle difference in semantics i.e. the reply taking place after the ISR is done. Finally, re dispatch table I wouldn't even be that concerned with performance plus if it's a tree like structure cmd(R/W)->index->subindex it should still be fairly efficient. Just toying ideas for now |
|
I would be concerned about changing the context ProcessStandardCommands is run in. I thought it was a good move to do that in the same context as terminal commands are executed. I don't think there's any problems but it does reduce the possibility of bugs depending on the context of execution. |
2 participants
This exports the list of errors and time at which they occurred as two CANopen SDO arrays. The sub-
index is used as an index into the list of errors. Any invalid indices return zero.
Clients can use the "lasterr" parameter database
spot-value which is present in all libopeninv projects to map the error number to a string.
Tests: