Skip to content

Replace internal network state objects with zigpy.state #85

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

Merged
merged 16 commits into from
Oct 29, 2021

Conversation

puddly
Copy link
Collaborator

@puddly puddly commented Aug 28, 2021

The only feature zigpy.state lacks right now is a way to keep track of device NWK and IEEE addresses, like in https://github.com/zigpy/open-coordinator-backup/blob/main/samples/z2m-sample-1.json#L53-L56.

Once that is done, the entire Z-Stack coordinator state can be fully represented by the new zigpy.state types. While this is not strictly necessary with #73, I've not yet tested the side effects of restoring a network backup with the child table completely erased.

My idea is to have zigpy behave something like this in the future and requiring radio libraries to implement only load_network_info and update_network_info:

async def form_network(self, *, force=False):
    # Or maybe do this inside of `startup()`?
    try:
        await self.load_network_info()
    except ValueError:
        pass

    # Do nothing if the `nwk_update_id` has not changed
    if not force and self.state.network_information.nwk_update_id == self.config[NWK_UPDATE_ID]:
        return

    # Otherwise, sync the network settings with what's in the zigpy config
    # XXX: how do you synchronize YAML config changes with ones done within ZHA's UI???
    await self.update_network_info()

@codecov-commenter
Copy link

codecov-commenter commented Aug 28, 2021

Codecov Report

Merging #85 (c5ce329) into dev (6bcd7f4) will increase coverage by 0.04%.
The diff coverage is 96.30%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev      #85      +/-   ##
==========================================
+ Coverage   98.71%   98.75%   +0.04%     
==========================================
  Files          44       44              
  Lines        3727     3768      +41     
==========================================
+ Hits         3679     3721      +42     
+ Misses         48       47       -1     
Impacted Files Coverage Δ
zigpy_znp/tools/common.py 100.00% <ø> (ø)
zigpy_znp/types/structs.py 100.00% <ø> (ø)
zigpy_znp/znp/security.py 94.24% <94.02%> (+1.54%) ⬆️
zigpy_znp/api.py 98.17% <95.20%> (-1.83%) ⬇️
zigpy_znp/tools/network_backup.py 97.95% <95.83%> (-2.05%) ⬇️
zigpy_znp/const.py 100.00% <100.00%> (ø)
zigpy_znp/tools/network_restore.py 100.00% <100.00%> (ø)
zigpy_znp/types/named.py 100.00% <100.00%> (ø)
zigpy_znp/zigbee/application.py 96.53% <100.00%> (+0.62%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6bcd7f4...c5ce329. Read the comment docs.

@Adminiuga
Copy link
Contributor

I was on the fence whether to keep the NWK address of the node for key. Is this device list actually for the devices, like "children" or is this the list of "APS Link keys" for APS encrypted communication?
If for later, the https://github.com/zigpy/zigpy/blob/fb71d982bfff6bcd96c4c046c8637295802d190f/zigpy/state.py#L59 the Key has partner_ieee, similar to APS key table structure ezsp keeps internally.
in other words, for the key table an attempt was made to keep the bare minimum if the purpose of this table is to store APS Link Keys.
Let me know what you think

@puddly
Copy link
Collaborator Author

puddly commented Aug 29, 2021

Is this device list actually for the devices, like "children" or is this the list of "APS Link keys" for APS encrypted communication?

It's a list of both, which is ambiguous now that I think about it (i.e one can't distinguish a child with an APS link key from a non-child).

I'm currently populating the key_table as you described (https://github.com/zigpy/zigpy-znp/pull/85/files#diff-963ec1db33f28713991ce851c9e14363cf2422d7ab7c68b4acd73ab35475549cR142) but to represent the entire backup state, we'd need some way to keep track of children. Z-Stack's device tables require both the IEEE and NWK addresses (but I think EmberZNet only really needs the IEEE?).

What about keeping a list of devices like this?

@dataclass
class NetworkDevice:
    ieee: t.EUI64
    nwk: t.NWK | None  # I'll have to test how to get Z-Stack to deal with these
    key: Key | None
    is_child: bool

@Adminiuga
Copy link
Contributor

does ZNP restoration need children or FFDs too?

What about keeping a list of devices like this?

I think that would work. Any suggestions how to call the list of these in app state? devices is a bit ambiguous, cause I don't want to keep all devices on the network there. neighbor_list maybe?

@puddly
Copy link
Collaborator Author

puddly commented Aug 29, 2021

does ZNP restoration need children or FFDs too?

The only devices present in my Z-Stack coordinator backup are:

  • Aqara sensors
  • IKEA routers with APS link keys

So it looks like only devices with APS link keys and RFD children. They all require NWK and IEEE addresses.

Any suggestions how to call the list of these in app state?

Since a device can be a "partner" with a key and/or a child, maybe immediate_family 😄? What about having two lists, one for children (with or without keys), and another for just partner devices with only keys? That would remove the need for the is_child attribute. On the other hand, that attribute is useful to have directly attached to the network info device object...

Maybe related_devices? partners? I'm okay with just devices and a comment explaining that it's not every single device.

@Adminiuga
Copy link
Contributor

Yeah, i think having two different lists is a better approach

@puddly
Copy link
Collaborator Author

puddly commented Sep 18, 2021

I've almost got this working with ZNP. form_network is independent of Z-Stack and can be moved into zigpy if other radio libraries replace form_network with write_network_info and load_network_info:

    async def form_network(self):
        """
        Clears the current config and forms a new network with a random network key,
        PAN ID, and extended PAN ID.
        """

        channel = self.config[conf.CONF_NWK][conf.CONF_NWK_CHANNEL]
        channels = self.config[conf.CONF_NWK][conf.CONF_NWK_CHANNELS]
        pan_id = self.config[conf.CONF_NWK][conf.CONF_NWK_PAN_ID]
        extended_pan_id = self.config[conf.CONF_NWK][conf.CONF_NWK_EXTENDED_PAN_ID]
        network_key = self.config[conf.CONF_NWK][conf.CONF_NWK_KEY]

        if pan_id is None:
            pan_id = random.SystemRandom().randint(0x0000, 0xFFFE + 1)

        if extended_pan_id is None:
            extended_pan_id = ExtendedPanId(os.urandom(8))

        if network_key is None:
            network_key = t.KeyData(os.urandom(16))

        # Override `channels` with a single channel if one is explicitly set
        if channel is not None:
            channels = t.Channels.from_channel_list([channel])

        network_info = zigpy.state.NetworkInformation(
            extended_pan_id=extended_pan_id,
            pan_id=pan_id,
            nwk_update_id=self.config[conf.CONF_NWK][conf.CONF_NWK_UPDATE_ID],
            nwk_manager_id=0x0000,
            channel=channel,
            channel_mask=channels,
            security_level=5,
            network_key=zigpy.state.Key(
                key=network_key,
                tx_counter=0,
                rx_counter=0,
                seq=0,
                partner_ieee=None,
            ),
            tc_link_key=None,
            key_table=[],
            neighbor_table=[],
            stack_specific={"zstack": {"tclk_seed": os.urandom(16).hex()}},
        )

        node_info = zigpy.state.NodeInfo(
            nwk=0x0000,
            ieee=None,
            logical_type=zdo_t.LogicalType.Coordinator,
        )

        await self.write_network_info(network_info=network_info, node_info=node_info)

Thoughts?

@puddly
Copy link
Collaborator Author

puddly commented Sep 18, 2021

The main problem with this general approach are radio quirks:

  • Current builds of Z-Stack can only hold four unhashed link keys.
  • EmberZNet doesn't allow for the coordinator IEEE address to be changed more than once. Is this a firmware limitation? Is it possible to change this behavior? Also, is the coordinator's IEEE address ever really used during normal network operation by other devices?
  • The deCONZ serial protocol has no public command to change the coordinator's IEEE address. There is definitely a way because deCONZ can with a backup/restore. No idea about APS link keys.
  • XBee: ???
  • ZiGate: ???

@Adminiuga
Copy link
Contributor

I like the idea.

Is this a firmware limitation? Is it possible to change this behavior?

Not really possible to change this. Has to do with the flash page erasing

Need to check deconz, IIRC you can update ieee address in deCONZ, but couldn't find anything in official documentation.

Zigate -- need to check this one.

@puddly
Copy link
Collaborator Author

puddly commented Oct 28, 2021

Updated to work with zigpy@dev.

At this point I'm only converting to JSON for presentation, all other operations use the zigpy state objects:

  • ControllerApplication.load_network_info(load_devices=False). Z-Stack can load network and node info in about a second if you exclude keys and devices, but it takes 10x longer if you include them. I think bellows behaves similarly when scanning the key tables and NWK address cache.
  • ControllerApplication.write_network_info(network_info, node_info)

Once a zigpy point release is made, we can:

  • move zigpy_state_to_backup_json and json_backup_to_zigpy_state into zigpy itself.
    • Alternatively, we could put them into zigpy-cli and just pickle the state objects when storing them in the database? This would make the database less readable, however, and require a command line tool to parse these entries.
  • move the generic network formation code into zigpy.
  • work on migrating existing radio libraries to support the load/write API (or whatever you want to call it).
  • see what we can do about getting this network info into ZHA in some advanced section? Maybe even adding a simple text box to copy/paste network state until we finalize a way to show all of the individual settings in the UI?

@Adminiuga
Copy link
Contributor

Do you need just zigpy release or pushed to HA too?

@puddly
Copy link
Collaborator Author

puddly commented Oct 28, 2021

Doesn't really matter, updated versions of both packages will probably go in the same PR to HA core.

@puddly
Copy link
Collaborator Author

puddly commented Oct 28, 2021

Sorry, I think I misread your comment. Just a zigpy release, so I can specify the minimum version in setup.py.

@puddly puddly force-pushed the puddly/centralized-network-state branch from a761d04 to 4b68155 Compare October 29, 2021 02:50
@puddly puddly merged commit 06af078 into zigpy:dev Oct 29, 2021
@puddly puddly deleted the puddly/centralized-network-state branch October 29, 2021 17:17
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

Successfully merging this pull request may close these issues.

3 participants