-
Notifications
You must be signed in to change notification settings - Fork 20
Description
When sending a directed long message through Node.send_parameter_group, the resulting transfer is always broadcast. j1939.Bus.send is always seeing a destination address of 0xFF.
I added some logging in Node.send_parameter_group immediately prior to the send:
logger.debug('send_parameter_group: to address: %s', pdu.arbitration_id.pgn.pdu_specific)
logger.info("DKP: send_parameter_group: is_destination_specific={}, destAddr={}".format(pdu.arbitration_id.pgn.is_destination_specific, pdu.arbitration_id.destination_address))
When sending a long message that requires the use of the transport protocol, I see the following results:
DEBUG:j1939:send_parameter_group: to address: 176 <---- correct address
DEBUG:j1939:PGN is_pdu1 00ef: True
DEBUG:j1939: (self.pdu_format & 0xFF) < 240 00ef: True
DEBUG:j1939: self.reserved_flag 00ef: 0
DEBUG:j1939: self.data_page_flag 00ef: 0
DEBUG:j1939:value-result = 00efb0
DEBUG:j1939:PGN is_destination_specific efb0: True
DEBUG:j1939:PGN is_pdu1 00ef: True
DEBUG:j1939: (self.pdu_format & 0xFF) < 240 00ef: True
DEBUG:j1939: self.reserved_flag 00ef: 0
DEBUG:j1939: self.data_page_flag 00ef: 0
DEBUG:j1939:value-result = 00efb0
DEBUG:j1939:PGN is_destination_specific efb0: True
INFO:j1939:DKP: j1939.send: is_destination_specific=True, destAddr=255 <---- global address
I believe the root issue is in ArbitrationID, where destination_address_value is maintained separately from pgn.pdu_specific, allowing them to hold different values that I believe should never be different. Node.send_parameter_group sets pdu.arbitration_id.pgn.pdu_specific to the destination_address, but not pdu.arbitration_id.destination_address.
My gut feel is that ArbitrationID.destination_address_value should just be a property that defers to pgn.pdu_specific. I see the following in ArbitrationID.__init__, which kinda indicates the separation may be intentional, and the commented assert makes me think the intention was that they should always be the same (yet it's been commented out...). I'm not sure if there's more to this than I'm seeing.
if self.destination_address_value != self._pgn.pdu_specific:
logger.debug("self._pgn=%s, self.destination_address_value = %x, pgn.pdu_specific = %x" %
(self._pgn, self.destination_address_value, self._pgn.pdu_specific))
# assert( self.destination_address_value == pgn.pdu_specific)
My workaround is to assign the destination address to both within Node.send_parameter_group, which gets me by for now, but isn't a fix, IMO:
pdu.arbitration_id.pgn.pdu_specific = pdu.arbitration_id.destination_address = destination_address
I'm happy to take a stab at a fix, but would need some guidance/clarification on the reason for separate address storage.