Open somlioy opened 1 year ago
Very technically, primask shall always be stored as local variable. Can you point out how your call stack looks like when this happens?
Or maybe disable CO_CONFIG_FLAG_SDO_ALWAYS_LOCK_OD
instead?
Yeah primask stored in a local variable would be the optimal choice for such an event, altought I'm not sure how that would be done when using macros like that. A local variable inside the do-while would be limited to access within that scope.
See Call stack below (breakpoint at the point where I call OC_LOCK_OD (in storeEeprom).
So CO_LOCK_OD is first called by CO_SDOserver_process() and then by me in storeEeprom()
I've already updated CANopenNode to latest version, so CO_CONFIG_FLAG_SDO_ALWAYS_LOCK_OD is deprecated. See commit attached in first post.
Technically I guess it safe to assume storeEeprom() and restoreEeprom() never would be called from anything else than via SDO such that the LOCKs I have in store-/restoreEeprom safely can be removed.
However there's still the "issue" where systicks are disabled doing a LOCK causing HAL_Delay to be stuck in a while-loop. Encountered this during my first implementation of the eeprom-writing function where HAL_Delay was used to allow for eeprom-page writing to be finished, thought this was fixed by polling the eeprom wether it is ready or not (HAL_I2C_IsDeviceReady).
I'm seeing this problem is well.
CO_LOCK_OD has a disable_irq() function inside: `#define CO_LOCK_OD(CAN_MODULE) \ do { \ (CAN_MODULE)->primask_od = get_PRIMASK(); \ __disable_irq(); \ } while (0)`
Where as CO_UNLOCK_OD only set the mask:
#define CO_UNLOCK_OD(CAN_MODULE) __set_PRIMASK((CAN_MODULE)->primask_od)
Should there be an enable irq inside CO_UNLOCK_OD ?
No. __set_PRIMASK will enable the interrupts, if these have been enabled before __disable_irq() has been called.
Ok, somehow the __set_PRIMASK is not re-enabling the interrupts for me.
This will happen if this happens:
CO_LOCK_OD();
CO_LOCK_OD(); <--- previous value is overwritten
CO_UNLOCK_OD(); <--- invalid previous value
CO_UNLOCK_OD(); <--- invalid previous value
So is there a case when LOCK is called twice in a sequence
Hi
After the update in CanopenNode where OD is always locked on a SDO call (Always call OD lock macros on SDO operations), my code broke down; the interrupts isnt enabled again.
This change caused nested OC_LOCK_OD to happen and since there is only one variable for storing the primask in the CO_CANmodule_t struct, the interrupts will never be enabled again.
The result was that the code stopped working on a HAL_Delay (infinite while loop) in anothert part of the application since OC_LOCK/__disable_irq apparantly also disable systicks.
I noticed this during a eeprom save (0x1010) because I had OC_LOCK_OD/UNLOCK on the OD-entry to be saved in the "storeEeprom"-function.
Is there any better way to implement this? Ie. not disabling global interrupts, only the timer in charge of the interrupts and some other way of storing primask?