Split between packages?

[cdjackson]

Hey Tommi. It's been a few months since I looked over this and you've moved up to V3 which is cool ;).

Just looking at the split between the Dongle and API projects and there seems to be a lot of stuff in the dongle project that's not dongle related (eg the cluster libraries seem to be there).

Shouldn't there be a cleaner split between the zigbee layers, so that the API and implementation of the cluster library is in one project, and the definition of the dongle, and implementation of the dongle are in separate projects?

Currently, it doesn't appear to me that it's possible to implement a different dongle without reuse of the 2531 project.

Maybe I misunderstand how it's now meant to work - any guidance?

Cheers
Chris

Edit: Migrated to https://github.com/zsmartsystems/com.zsmartsystems.zigbee

[tlaukkan]

Hi

Nice to have someone to take a look in the latest changes in detail :).

Unfortunately the old implementation is so dangled that it is not really
feasible to separate different bits and pieces. The new API has a cleaner
abstraction where java pojos represent command messages and the dongle
interface itself is quite simple. The legacy implementation is dumped to
the dongle project. There old jars and API exist for backwards
compatibility and Readme.md describes the new API & dependencies.

I have another dongle in my shelf from another vendor which I plan on
integrating. In this process I hopefully can create some parts which can be
reused when implementing further dongles.

If you dig into the new API you can see the new implementation of command
messages in ZigBee common.

If you find any inconsistencies or things need refactoring lets chat and
bumb them around :).

Br,
Tommi

PS. Here are some key intefaces:

package org.bubblecloud.zigbee.v3;

/**

• ZigBee dongle interface implemented by different dongle hardware drivers.
  /
  public interface ZigBeeDongle extends ZigBeeNetwork {
  /*

  • Starts up dongle.
    *
  • @return true if startup was success.
    */
    boolean startup();

  /**

  • Shuts down dongle.
    */
    void shutdown();
    }

package org.bubblecloud.zigbee.v3;

/**

• ZigBee network.
  /
  public interface ZigBeeNetwork {
  /*
  • Sends ZigBee Cluster Library command without waiting for response.
  • @param command the command
  • @return transaction ID
  • @throws ZigBeeException if exception occurs in sending
    /
    int sendCommand(final Command command) throws ZigBeeException;
    /*
  • Adds ZigBee Cluster Library command listener.
  • @param commandListener the command listener
    /
    void addCommandListener(final CommandListener commandListener);
    /*
  • Removes ZigBee Cluster Library command listener.
  • @param commandListener the command listener
    */
    void removeCommandListener(final CommandListener commandListener);
    }

On Sat, Aug 20, 2016 at 11:55 AM, Chris Jackson notifications@github.com
wrote:

Hey Tommi. It's been a few months since I looked over this and you've
moved up to V3 which is cool ;).

Just looking at the split between the Dongle and API projects and there
seems to be a lot of stuff in the dongle project that's not dongle related
(eg the cluster libraries seem to be there).

Shouldn't there be a cleaner split between the zigbee layers, so that the
API and implementation of the cluster library is in one project, and the
definition of the dongle, and implementation of the dongle are in separate
projects?

Currently, it doesn't appear to me that it's possible to implement a
different dongle without reuse of the 2531 project.

Maybe I misunderstand how it's now meant to work - any guidance?

Cheers
Chris

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#93, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAONRq-EIoAqCmYbGKIrB5WYjLHpVKEOks5qhsD3gaJpZM4JpCHp
.

Best regards,
Tommi Laukkanen

[cdjackson]

Hi Tommy,
I’ve not spent too much more time looking at this yet, but from what I can tell the zigbee-common project (which I guess is the main ‘api’) is a thin wrapper that doesn’t do much (other than to define the API I guess).

I would have thought that the ‘common' library should contain all the management functions and tope level handling of the ZCL and maybe ZDO - I guess the ZDO could be abstracted away if it was deemed to useful to do so. The dongle library should then only contain the minimal required to package up the packets from the common management layers.

At the moment, the management functions at least are still in the dongle - actually, from what I can tell (from my quick look) most of the implementation is in the dongle…

The dongle project also seems to have a lot of stuff associated with the cluster library which I don’t understand - I would have expected all of the cluster library implementation to be in the common parts?

Maybe this is where you’re heading when you start to look at other dongles? Or maybe I’m off the mark and it’s better to put more into the dongle for some reason? I’ve also looked at the ember (Telegesis dongle with the AT command set - even if I’m not especially keen on AT style ASCII packets!) so am keen on this refactoring.

What is the other dongle you are looking at?

Cheers
Chris

[tlaukkan]

Hi

You just need to implement the two interfaces to startup, shutdown and
transmit commands. The existing dongle implementation contains lots of
legacy code which is not used with the 3.0 api.

Br,
Tommi

On 22 Aug 2016 00:43, "Chris Jackson" notifications@github.com wrote:

Hi Tommy,
I’ve not spent too much more time looking at this yet, but from what I can
tell the zigbee-common project (which I guess is the main ‘api’) is a thin
wrapper that doesn’t do much (other than to define the API I guess).

I would have thought that the ‘common' library should contain all the
management functions and tope level handling of the ZCL and maybe ZDO - I
guess the ZDO could be abstracted away if it was deemed to useful to do so.
The dongle library should then only contain the minimal required to package
up the packets from the common management layers.

At the moment, the management functions at least are still in the dongle -
actually, from what I can tell (from my quick look) most of the
implementation is in the dongle…

The dongle project also seems to have a lot of stuff associated with the
cluster library which I don’t understand - I would have expected all of the
cluster library implementation to be in the common parts?

Maybe this is where you’re heading when you start to look at other
dongles? Or maybe I’m off the mark and it’s better to put more into the
dongle for some reason? I’ve also looked at the ember (Telegesis dongle
with the AT command set - even if I’m not especially keen on AT style ASCII
packets!) so am keen on this refactoring.

What is the other dongle you are looking at?

Cheers
Chris

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#93 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAONRlGwhchqQl-Z47mEU_xVJrri9eZrks5qiMZ1gaJpZM4JpCHp
.

[cdjackson]

Hi Tommi,
I’ll spend a bit more time trying to get my head around this.

Can you tell me what other dongle you’re looking at supporting?

Cheers
Chris

On 22 Aug 2016, at 07:56, Tommi S.E. Laukkanen notifications@github.com wrote:

Hi

You just need to implement the two interfaces to startup, shutdown and
transmit commands. The existing dongle implementation contains lots of
legacy code which is not used with the 3.0 api.

Br,
Tommi

On 22 Aug 2016 00:43, "Chris Jackson" notifications@github.com wrote:

Hi Tommy,
I’ve not spent too much more time looking at this yet, but from what I can
tell the zigbee-common project (which I guess is the main ‘api’) is a thin
wrapper that doesn’t do much (other than to define the API I guess).

I would have thought that the ‘common' library should contain all the
management functions and tope level handling of the ZCL and maybe ZDO - I
guess the ZDO could be abstracted away if it was deemed to useful to do so.
The dongle library should then only contain the minimal required to package
up the packets from the common management layers.

At the moment, the management functions at least are still in the dongle -
actually, from what I can tell (from my quick look) most of the
implementation is in the dongle…

The dongle project also seems to have a lot of stuff associated with the
cluster library which I don’t understand - I would have expected all of the
cluster library implementation to be in the common parts?

Maybe this is where you’re heading when you start to look at other
dongles? Or maybe I’m off the mark and it’s better to put more into the
dongle for some reason? I’ve also looked at the ember (Telegesis dongle
with the AT command set - even if I’m not especially keen on AT style ASCII
packets!) so am keen on this refactoring.

What is the other dongle you are looking at?

Cheers
Chris

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#93 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAONRlGwhchqQl-Z47mEU_xVJrri9eZrks5qiMZ1gaJpZM4JpCHp
.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub #93 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/AA_kQ259swZI1Cjsbhz02jQ047hhJvoCks5qiUgngaJpZM4JpCHp.

[tlaukkan]

Hi Chris

I am going to look into NXP / Kinetis USB-KW2X:

http://www.nxp.com/files/rf_if/doc/ref_manual/USB-KW2XHWRM.pdf

Br,
Tommi

[cdjackson]

Thanks Tommi,
I must say the NXP devices look nice - there are some nice embedded modules around. I'm certainly keen to look at implementing another dongle so might take a look at the Telegesis as it's also widely available. I just wanted to make sure we weren't going to do the same thing ;)

I spent a bit more time looking at the library and started porting some code so force me to look at the implementation - I have a few comments/questions on the overall structure that I'd welcome your thoughts on...

Cheers
Chris

Maybe I’ve misunderstood, but the API seems quite ‘flat’. From what I can see, everything is performed at the ZigBeeApi level - so we have api.On() and api.Off() - and all the other main functions we might want to support. For attributes, we seem to have a single ZclAttributeType file which seems to list all the attributes from all clusters (I’ve not checked, but this is how it looks?).

So, if I understand, to turn on a switch, and then get its state (at a later time), I do -:

api.On()
…
api.read(clusterID, attrId);

Does it not make more sense to perform actions on clusters - i.e. to have a cluster class like the old library? So, we would do -:

cluster = api.getCluster(clusterId)

cluster.On()
cluster.getState();

The flat approach might be simple in one respect (it’s simple for many applications), but it doesn’t seem very maintainable or modular since everything needs to be done at the API level and not broken down into lower layers of functionality. It just doesn’t feel like the best approach in an object oriented world ;)

What do you think? Is it worth/possible to provide such an interface? I know it could be added over the top of this interface so that’s one option I could look at if you’re not keen. I haven't looked at the code generator in any real detail, but I guess all this is generated automatically, so it could be implemented there to generate the alternative API?

———

We seem to have lost the concept of the physical device (what was the node in V2). As such, I don’t see that we can get device level information like the ZigBeeNodeDescriptor and ZigBeeNodePowerDescriptor? Is this still possible in the new API?

[cdjackson]

Hi Tommi,
Any thoughts on my comment above on the API?

I've significantly extended the code generator now to add support for attributes as well as commands. It now generates classes for clusters, and getters/setters for attributes, as well as adding javadoc from the definition file. I've also refactored the definition file to make it a markdown file (which it very nearly was anyway) so that it can also be read nicely.

I'd appreciate your comments though - if this is going in a different direction than you see this project then I need to rethink ;)

Cheers
Chris

[tlaukkan]

Hi Good idea and nice to here that API side is moving forward. Could you post some examples of latest developments with attribute getters and setters. I rather like that the device id is given as parameter instead of having separate cluster api instances for each device. This would work though: final OnOffCluster onOffCluster = api.getOnOffCluster(); onOffCluster.on(deviceId); We should keep the client stateless to keep things simple and then per device instances are not required. Br, Tommi

[cdjackson]

Hi Tommi,
I’ve just pushed my refactoring to this branch https://github.com/cdjackson/zigbee4java/tree/cluster-autogeneration https://github.com/cdjackson/zigbee4java/tree/cluster-autogeneration

It all seems to be running with the existing API. My next step is to try and port over my existing code (which uses the V2 library) to use the new V3 cluster API.

I’ve refactored things quite a bit (sorry) so please feel free to comment if you don’t like something ;). My original intention had been to just add the cluster classes (hence the name of the branch!) but it then seemed to make sense to move the commands classes and I ended up with quite a few changes…. Comments welcome ;)

Cheers
Chris

[cdjackson]

Hey Tommi,
Regarding your concern about having multiple device instances, I guess your concern is there’s a lot of information stored in the device and replicating that might be a bit heavy?

Currently, we have a ZigBeeDestination, which is either a groupID, or a device. Does ZigBeeDevice need to extend the destination (??) - maybe we should have a ZigBeeAddress class which olds the network address, and the endpoint id? We could then use this where full device information is not needed (like creating cluster classes). It would also allow us to reduce the following code to a single line -:

        command.setDestinationAddress(((ZigBeeDevice) destination).getNetworkAddress());
        command.setDestinationEndpoint(((ZigBeeDevice) destination).getEndpoint());

Just thinking out loud…

Chris

[tlaukkan]

Hi

Granted the current ZigBeeDevice in the new API is quite good as it contains just the addressing and basic characteristics. Would change the example as follows:

final OnOffCluster onOffCluster = api.getOnOffCluster();
onOffCluster.on(device);

Another style would be:

api.onOffCluster().on(device);

This approach does not follow the established coding conventions so it makes me little hesitant...

I find it better design to separate value objects and logic layer like this so you do not need to upkeep all connectivity related references and code in the value objects. If you analyse the old implementation you can see that mixing value objects and logic layer can lead to quite convoluted codebase.

ZigBeeDestination is abstract base class to allow targeting of both individual ZigBee endpoints and ZigBee groups with same API methods which keeps the number of code lines to minimum. Even if we code generate a lot of things it is good to keep things compact.

Br,
Tommi

[tlaukkan]

Your cluster code generation looks quite neat. How would one use them from API level? I guess the basic question is whether from the final API use perspective it is more convenient to have:

api.getOnOffCluster(address).on();

or

api.getOnOffCluster().on(address);

or

api.onOffCluster().on(address);

I don't think there is performance difference or code compactness question here so it is a question of style. Creating the cluster classes is matter of convenience for API users right? You could also just send and observe the command classes directly but that takes quite many code lines.

By the way not all commands are ZCL commands where you invoke things through clusters so we need to figure out how to access them as well in similar manner:

api.zdo().bind(...)

What do you think and can you find further ways of making the API optimal for sending commands and manipulating attributes?

Br,
Tommi

[cdjackson]

Hi Tommi,
Currently, in my implementation I create the cluster, and then use it in many places within a (kind of) converter class that converts between ZigBee, and the framework code. You are right though that it’s a question of style and you could use the command classes directly - no problem. I think the clusters provide a more structured view, but just my preference really.

Over the coming few weeks I’m travelling a lot so hope to spend some ‘hotel time’ looking at this and implementing another dongle. I was also thinking about adding a ZclAttribute class to improve the use of attributes, but I’ve not really thought this through well enough yet.

The code generator does make it easy to consider different interfaces and then generate 50 classes in different implementation though - really useful when considering different interfaces :)

Edit: Migrated to https://github.com/zsmartsystems/com.zsmartsystems.zigbee

Chris