ZDO_SIMPLE_DESC_RSP timeout about zigbee4java HOT 7 CLOSED

It looks like the following is causing it in ApplicationFrameworkLayer -:

        final int[] clusters = new int[clusterSet.size()];
        if (clusters.length > 33) {
            throw new RuntimeException("Too many default clusters.");
        }

If I change this to 34 and add another cluster to the cluster set as per your change to add the measurement cluster, then it stops working properly...

from zigbee4java.

Ahh, yes cleared my persistence and I did see that error. I think I have found the reason, too. The ZDO_SIMPLE_DESC_RSP only supports 33 clusters. This seems to be a limitation of the TI Z-Stack Monitor API.

We could just change it back to 33, but the problem there is that these new clusters can't be bound, and therefore no attribute reporting. It's only going to get worse as we add support for more clusters.

zigbee4java doesn't allow access to any endpoints of node #0, as that is the coordinator node. Let me figure out what it actually uses this information for. Have you found any problems other than the error message?

from zigbee4java.

Z-Stack also limits the AF_REGISTER to 33 clusters. I don't see why we can't just create a new endpoint when we run out. I'll give this a shot.

As an aside while looking at the code I think I see another bug. Here is the code that deals with the coordinator endpoints. It looks like it registers the output clusters if it's a coordinator, and if not it adds the endpoint to the network list.
https://github.com/tlaukkan/zigbee4java/blob/master/zigbee-api/src/main/java/org/bubblecloud/zigbee/network/discovery/EndpointBuilder.java#L157-L166

However the code here also registers the output clusters. It looks to me like it ends up being done twice.
https://github.com/tlaukkan/zigbee4java/blob/master/zigbee-api/src/main/java/org/bubblecloud/zigbee/network/impl/ApplicationFrameworkLayer.java#L202

from zigbee4java.

With this commit b380e13 I no longer see the problem. Let me know if you have any more issues with it.

It also allows for greater than 33 clusters by using multiple endpoints. I'll look at the other potential bug later, to see if it's really an issue.

from zigbee4java.

Great- that fixes it thanks. I don't notice any other issues at the moment, but my network is small (I'm currently in a hotel as I'm travelling for work so have just grabbed a couple of devices to play with :) ).

Can I make a suggestion - I don't think developing directly into the master branch is a wise move. I think we should develop in separate branches and then create a PR to pull across the changes so that someone else can have the chance to comment on the changes... It's not related to this change as I suspect no-one would have noticed it, but I think it's good to have someone review changes for a bit of a sanity check...

Just a thought :)

from zigbee4java.

Heh, yeah I thought you might be thinking as such, knowing your style of a pull request for small changes. ;) I know I asked @tlaukkan a while ago if changing master was preferred vs a branch, and at least at the time he said going right to master was fine.

In the case of this issue the changes to the existing code was pretty small (just a few lines), but one of those lines caused the error above. One solution would be to create a work branch where the unstable changes live until they are tested/commented on and then moved to a stable branch eventually.

I'm not sure the overhead of numerous pull requests is appropriate at this point... If the project were more mature and it was used in production then that might be more appropriate. Even for commits though anyone can comment on them and things can be changed, so it's not like using commit vs pull request doesn't allow code review.

Let's see what @tlaukkan thinks about all this. I think something like a work branch would isolate the master and allow us to review the code before merging back into master. Using pull requests seems like unnecessary overhead at this stage of development. But whatever Tommi thinks is appropriate is fine by me.

from zigbee4java.

Hey Ryan,
I was just thinking of trying to get some sort of review for changes - at least anything significant. Again, this isn’t related to this change - I just thought I’d raise it :) I don’t suggest to change the way anyone codes and commits, but as there are a few of us working on this it just seemed like a good idea to avoid / reduce any screwups… I do the PRs to make sure others have visibility of the changes before they’re merged, otherwise you need to go through all the commits to find what’s changed…

It was just a thought aimed at providing a level of review to keep up the code quality - I personally think it helps for anything more than a few lines (not just for changing timer values or comments etc) but if others think it’s not needed them I’m fine with that…

Cheers
Chris

from zigbee4java.