Bug #283

Manufacturer ID must be sent in reverse byte order

Added by Philipp Buluschek almost 4 years ago. Updated almost 4 years ago.

Status:Feedback Start date:11/27/2014
Priority:Low Due date:
Assignee:Stefano Lenzi % Done:

100%

Category:zigbee.zcl.library Spent time: -
Target version:org.aaloa.zb4osgi.zigbee.zcl.library-0.9.0
Has a patch:No Has license agreement signed:No

Description

In ZCLHeaderImpl, the manufacturer ID must be sent in reverse byte order.

private byte[] createHeader(){
  ...
  newHeader[1] = manufacturerId[1];
  newHeader[2] = manufacturerId[0];
  ...
}

Associated revisions

Revision 1113
Added by Stefano Lenzi almost 4 years ago

Updating Javadoc for property ( fixes #283 )

History

#1 Updated by Stefano Lenzi almost 4 years ago

  • Category set to zigbee.zcl.library
  • Status changed from New to In Progress
  • Assignee set to Stefano Lenzi
  • Priority changed from Normal to Low
  • Target version set to org.aaloa.zb4osgi.zigbee.zcl.library-0.9.0

At the moment it is not used at all in the code, but considering that we are using a byte[] then the data should arrived in the proper order. Otherwise we should change byte[] to int and the serialize and deserialize the data. I would prefer the switch to int because it is clearer for the developers.

What do you think?

#2 Updated by Stefano Lenzi almost 4 years ago

  • Status changed from In Progress to Closed
  • % Done changed from 0 to 100

Applied in changeset r1113.

#3 Updated by Stefano Lenzi almost 4 years ago

  • Status changed from Closed to Feedback

Wrong commit message closed this issue by chance, please check the answer

At the moment it is not used at all in the code, but considering that we are using a byte[] then the data should arrived in the proper order. Otherwise we should change byte[] to int and the serialize and deserialize the data. I would prefer the switch to int because it is clearer for the developers.

What do you think?

#4 Updated by Philipp Buluschek almost 4 years ago

I'm currently using it, and so had to make the required changes...

I don't believe it is the caller's responsibility to know the ZigBee detail that data is sent byte-order reversed. When the ZigBee Alliance publishes a manufacturer ID, it is like 0x1234. Having the ID in the code as byte[]{0x34,0x12}; would be very counterintuitive.

On the other hand, the ID is always 16bit unsigned, so we should use a corresponding data type.

In AbstractCommand, a null manufacturer ID indicates that the command is not manufacturer specific. Although, in general, using null as a special-meaning value is ugly (IMHO), here it makes the code much simpler. All clients (the Command implementation, but also the manufacturer specific AttributeImpl and the corresponding Subscription) can just use one constructor and pass null for standard Commands/Attributes and pass the ID in the other case.
In short: having a null value to indicate standard conformance is convenient here - but so you cannot use a short or int instead. Maybe a DoubleByte or better still a domain specific "ManufacturerID" class would fit?

Also available in: Atom PDF