Skip to content

Order of operations in NmtMaster.on_heartbeat #579

@sveinse

Description

@sveinse

In the implementation of NmtMaster.on_heatbeat, is it deliberate that the callbacks are called before the _state of the class is updated and waiters notified? The emcy.py implementation is opposite., where it process the data, updates the class state, notify waiters and calls the callbacks.

canopen/canopen/nmt.py

Lines 122 to 137 in e840449

def on_heartbeat(self, can_id, data, timestamp):
with self.state_update:
self.timestamp = timestamp
new_state, = struct.unpack_from("B", data)
# Mask out toggle bit
new_state &= 0x7F
logger.debug("Received heartbeat can-id %d, state is %d", can_id, new_state)
for callback in self._callbacks:
callback(new_state)
if new_state == 0:
# Boot-up, will go to PRE-OPERATIONAL automatically
self._state = 127
else:
self._state = new_state
self._state_received = new_state
self.state_update.notify_all()

When adopting for async, these callbacks will be deferred for later execution. If it is expected that the callbacks run before the class state (like _state) is updated, I will need to adopt specially for that. Otherwise I propose changing the order in NmtMaster.on_heartbeat to mirror the order in EmcyConsumer.on_emcy.

@christiansandberg I see from git blame that this is your code. Your input would be much appreciated. Thanks.

edit:
The lock is held while the callbacks of on_heartbeat() is called, while on_emcy its not. Is that also of importance?

canopen/canopen/emcy.py

Lines 26 to 40 in e840449

def on_emcy(self, can_id, data, timestamp):
code, register, data = EMCY_STRUCT.unpack(data)
entry = EmcyError(code, register, data, timestamp)
with self.emcy_received:
if code & 0xFF00 == 0:
# Error reset
self.active = []
else:
self.active.append(entry)
self.log.append(entry)
self.emcy_received.notify_all()
for callback in self.callbacks:
callback(entry)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions