Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query about zbhub #55

Open
jrjefferies opened this issue Oct 29, 2017 · 2 comments
Open

Query about zbhub #55

jrjefferies opened this issue Oct 29, 2017 · 2 comments

Comments

@jrjefferies
Copy link
Contributor

I have spent most of the day working through the structure and trying to work out why we are messing up the joining. I think I have found something. The import structure goes like this (I think)
console-example -> zbhub -> zbnode -> node
Note - We can replace console-example with hub-example - it calls the same stuff

We we initiate the ZBHub, we initiate the ZBNode. This starts the zigbee library with a callback of 'receive_message'

receive_message does three things

  • parse_message (generated a list of messages to send)
  • process_message (we'll come back to this)
  • sends the messages generated by parse_message

The next thing to note is the zbnode process_message is documented as being STUB to be overwritten by zbhub
So in zbhub the process_message does the following

  • finds the device object (using device_obj_from_addr)
  • sets the device object attributes

Now the problem is that device_obj_from_addr does the following

  • create the node_obj if it does not exist
  • sends AlertmeCluster commands if the done_obj.type does not exist
    • generate_message (a zbnode procedure)
    • send_message (a zbnode procedure)

Because the device_obj_from_address is sending directly from the zbnode class, these message are

  • Sent before the parse_message generated messages
  • Send potentially too early

It looks like the messages being generated in device_obj_from_address are in the wrong place. This may be a leftover from an earlier version (sorry, not checked the commit record).

So we can do a couple of things. Move the send messages from the device_obj_from_addr (feels the correct thing to do), or move the process_message until after the parse_message queue is send.
I probably need guidance on what direction to go to..

Regards,
James

@jamesleesaunders
Copy link
Owner

jamesleesaunders commented Oct 29, 2017

@jrjefferies Another good spot!

I am glad you have managed to untangle my possibly confusing web of function names and inheritance. This I feel could possibly do with a little untangling once understood. The idea was that all ‘zb’ classes inherited from zbnode could be written in such a way that they perform different functions on receiving a message (I.e the hub creates a list of known end devices, whereas the SmartPlug simply records its hub address).

I haven’t got my kit up and running at the moment but from what you describe it looks like we are potentially sending discovery messages ( from device_obj_from_addr) before the expected replies therefore possibly confusing the end device? Or worse causing a cycle of repeating messages.

Like you say we have a couple of options...

  1. zbnode.py Swap the ‘Update any attributes’ and ‘Send any replies’ over to become:
def receive_message(self, message):
        """
        Receive message from XBee.
        Parse incoming message.
        Process parsed result.

        :param message: Dict of message
        :return:
        """
        ret = self.parse_message(message)

        if message['id'] == 'rx_explicit':
            source_addr_long = message['source_addr_long']
            source_addr_short = message['source_addr']

            # Send any replies which may need sending           <-- swapped over with 
            for reply in ret['replies']:
                message_id = reply['message_id']
                if 'params' in reply.keys():
                    params = reply['params']
                else:
                    params = {}
                reply = self.generate_message(message_id, params)
                self.send_message(reply, source_addr_long, source_addr_short)
                time.sleep(0.5)

            # Update any attributes which may need updating       <-- swapped over
            self.process_message(source_addr_long, source_addr_short, ret['attributes'])

Or

  1. zbhub.py Remove the send_message code from device_obj_from_addr() but we loose the sending of type discovery messages.
Remove:??
            # if not device_obj.type:
                # The device has to receive these two messages to stay joined.
                # message = self.generate_message('mode_change_request', {'mode': 'normal'})
                # self.send_message(message, device_addr_long, device_addr_short)
                # message = self.generate_message('version_info_request')
                # self.send_message(message, device_addr_long, device_addr_short)

To be honest I’d be interested in your view and your testing, but both sound plausible. The advantage of solution 1 is that it maintains the rudimentary force discovery of new unknown device types. I will attempt to get my kit setup again, but in the mean time, if you do come to a solution please feel free to pop it in a PR and I'll happily merge it in @andydvsn may also be interested.

All the best,

Jim

@jrjefferies
Copy link
Contributor Author

Jim,
I tried option 1 - moving the process_message. It looks better, the plug seemed to join, then went back to range finding mode. I intend try and identify what message the zbhub sent to cause it to step back. If I find a problem, we may be good.

If not, I am thinking about putting a statemachine in the zbhub process_packets. It will work something like this

  • new node_obj joining_status variable that will migrate through unknown -> zdo_stage1 -> zdo_stage2 -> Alertme_stage1 -> Alertme_stage2 -> fully_joined
  • pass message queue into the process_message, so we can add the messages we want
  • process_message will have if statements to add in additional message on the end of the message queue.
    E.g. when we get Match_Descriptor_request the zbnode will add Match_descriptor_response, and then we add Active_Endpoint_Request to trigger the switch to move to the next state
  • we could also reorder the packets in the message queue if needed.

[NOTE - stages to be worked out - I am still trying to get my head around all the steps needed for proper joining of the hub]

This would keep the zbnode parse_message for generic message responses and move the specific coordinator responses in the zbhub process_message

Tell me what you think.
Cheers James

PS - I will commit the changes I made on this issue - even though its not completely fixed the issue.
This includes to formatting changes to the zbnode static variables, correction on the short broadcast variable, and some documentation

jrjefferies added a commit to jrjefferies/PyAlertMe that referenced this issue Oct 30, 2017
jamesleesaunders added a commit that referenced this issue Nov 12, 2017
Update to issue #55 (not fully fixed), + document / readability changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants