niolabs / python-xbee

Python tools for working with XBee radios
MIT License
101 stars 45 forks source link

Minor refactoring due to what it seems a fail in the Digi Xbee documentation #1

Open LuisPinaSoares opened 8 years ago

LuisPinaSoares commented 8 years ago

What steps will reproduce the problem?

  1. Trying to issue an 'ND' command (API mode) and later parsing it (the problem is in the parsing). For example: xbee.send("at", frame='A', command='ND') print xbee.wait_read_frame()

What is the expected output? What do you see instead?

You were expected to receive the following information associated with the ND command refered in the datasheet:

QUOTE:

Node Discover. Discovers and reports all RF modules found. The following information is reported for each module discovered. MY SH SL NI (Variable length) PARENT_NETWORK ADDRESS (2 Bytes) DEVICE_TYPE (1 Byte: 0=Coord, 1=Router, 2=End Device) STATUS (1 Byte: Reserved) PROFILE_ID (2 Bytes) CRE MANUFACTURER_ID (2 Bytes)

If ND is sent through the API, each response is returned as a separate AT_CMD_Response packet. The data consists of the above listed bytes without the carriage return delimiters. The NI string will end in a "0x00" null character. The radius of the ND command is set by the BH command. END QUOTE: Instead it seems like the module sends you additional data (4 bytes) that are not refered in the datasheet. To me these 4 bytes seem to be additional info refering to the command 'DD' (Device identifier type). This makes the method "_parse_ND_at_response(self, packet_info)" send an Error message. **What version of the product are you using? On what operating system?** I m using the Xbee Zigbee Pro S2B, later i will test this with Xbee Zigbee S2B Please provide any additional information below. You can solve this adding the following line (in method _parse_ND_at_response(self, packet_info)): ``` "result['device_type_identifier'] = packet_info['parameter'][null_terminator_index+9:null_terminator_index+13]" ``` after the line: ``` "result['manufacturer'] = packet_info['parameter'][null_terminator_index+7:null_terminator_index+9]" ``` and modifying the test before return of the method to: ``` if null_terminator_index+13 != len(packet_info['parameter']): raise ValueError("Improper ND response length: expected {0}, read {1} bytes".format(len(packet_info['parameter']), null_terminator_index+9)) ``` The correct way to solve probably would be to take this last test out, and believe in the checksum. This way if Digi decides to add more information to the DATA packet you won t have problems and will only parse it if necessary --- **UPDATE** Additional information of the packets just to help understand: 1 DEVICE ------------------------- Frame data: 88014e440000000013a20040b0e9e34d41494e00fffe0000c105101e00030000 Frame raw_data: 7e002088014e440000000013a20040b0e9e34d41494e00fffe0000c105101e000300005a node_identifier 4d41494e parent_address fffe device_type 00 status 00 profile_id c105 manufacturer 101e device_type_identifier 00030000 {'status': '\x00', 'frame_id': '\x01', 'parameter': {'status': '\x00', 'source_addr': '\x00\x00', 'parent_address': '\xff\xfe', 'profile_id': '\xc1\x05', 'device_type_identifier': '\x00\x03\x00\x00', 'source_addr_long': '\x00\x13\xa2\x00@\xb0\xe9\xe3', 'device_type': '\x00', 'node_identifier': 'MAIN', 'manufacturer': '\x10\x1e'}, 'command': 'ND', 'id': 'at_response'} 2 DEVICE ------------------------- Frame data: 88014e44004b0e0013a20040c1bb0b20454e44320000000200c105101e00030000 Frame raw_data: 7e002188014e44004b0e0013a20040c1bb0b20454e44320000000200c105101e00030000ed node_identifier 20454e4432 parent_address 0000 device_type 02 status 00 profile_id c105 manufacturer 101e device_type_identifier 00030000 {'status': '\x00', 'frame_id': '\x01', 'parameter': {'status': '\x00', 'source_addr': 'K\x0e', 'parent_address': '\x00\x00', 'profile_id': '\xc1\x05', 'device_type_identifier': '\x00\x03\x00\x00', 'source_addr_long': '\x00\x13\xa2\x00@\xc1\xbb\x0b', 'device_type': '\x02', 'node_identifier': ' END2', 'manufacturer': '\x10\x1e'}, 'command': 'ND', 'id': 'at_response'}
hansmosh commented 8 years ago

Hi Luis, sorry for the late response, but hopefully I can be some help.

Am I correct in believing that this ValueError is raised? https://github.com/nioinnovation/python-xbee/blob/master/xbee/zigbee.py#L207

Your proposed fix sounds reasonable for this situation you're seeing (and I definitely agree that the checksum should be used in place of what's there), but I wonder why I don't see this behavior documented here: http://examples.digi.com/wp-content/uploads/2012/07/XBee_ZB_ZigBee_AT_Commands.pdf

Did you ever find any XBee documentation to back up what you are saying is a 4 byte device identifier?

LuisPinaSoares commented 8 years ago

Hi,

There is a Device Type identifier (DD) command in the AT commands (Addressing commands), page 128 of the document:

DD Device Type Identifier. Stores a device type value. This value can be used to differentiate different XBee-based devices. Digi reserves the range 0 - 0xFFFFFF. For example, Digi currently uses the following DD values to identify various ZigBee products: 0x30001 - ConnectPort X8 Gateway 0x30002 - ConnectPort X4 Gateway 0x30003 - ConnectPort X2 Gateway ... CRE 0 - 0xFFFFFFFF 0x30000

I think in the case of the ND command they added the DD (Device Type Identifier) to the answer payload (for some reason they might have started to have the need for that info), and because of that when the answer is parsed you get the, ValueError.

hansmosh commented 8 years ago

It concerns me that the documentation for ND doesn't mention anything about a 4 byte Device Type identifier (just the Device Type), but it does look like your analysis of the response frame you're getting makes sense.

Is there some new (undocumented) firmware that you are using? Or maybe it's specific to Pro S2B? Did you try with the regular S2B?

Given that the code fix you suggested works for you, I'm happy to review and merge a pull request that fixes the problem. Please write the code in such a way that a frame without those extra 4 bytes is still properly parsed.

LuisPinaSoares commented 8 years ago

OK. As soon as i am able to look into that, i will make the pull request.

jamesleesaunders commented 7 years ago

Hi @LuisPinaSoares did you get round to putting together a PR for this issue?

LuisPinaSoares commented 7 years ago

Hi James,

I havent.

The reason for this, is i later found out that the code didnt work because i changed the DD value on the coordinator, to a diferent value than the default.

When i changed back to a default value the problem was solved.

Later the diferent DD values should besuported in the code, but if you use a default value there will be no issue with the current code version. If you change the DD value to some non default values, the coordinator will give a new key in the ND response request dictionary, "device_type_identifier", which is not parsed currently.

Regards

On Wed, Aug 17, 2016 at 5:36 PM, James Saunders notifications@github.com wrote:

Hi @LuisPinaSoares https://github.com/LuisPinaSoares did you work out a code fix for this issue?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nioinnovation/python-xbee/issues/1#issuecomment-240470010, or mute the thread https://github.com/notifications/unsubscribe-auth/ALaje7Iv6sJaN_33xN-LrYh3zng2NzLdks5qgzh3gaJpZM4GUnO7 .