add mesh properties to host info

[AaronDavidSchneider]

Add mesh info to get_hosts_info. Note: Mac adresses do not match between get_specific_host_entry and get_mesh_topology. I am not sure if there is a work around it.

[chemelli74]

I did a test with this new get_hosts_info() and compared result with arp -a and MAC addresses have all a match.

Simone

[chemelli74]

Tested with both get_specific_host_entry() and get_specific_host_entry_by_ip() and they match as well.

Simone

[chemelli74]

Great job, works smoothly here.

What do you think @kbr ? Ready to merge ? Would also be great if after merging, you are so kind to publish a new release ;-)

Simone

[kbr]

Thank you very much for your time and ideas to enhance the library. Please let me provide my ideas by means of a code review.

Instead of scanning the topology information for every device I would suggest to build up a dictionary with the according informations. So the _get_mesh_by_mac function may be replaced by a get_mesh_topology_info method:

def get_mesh_topology_info(self, topology=None):
    """
    Returns the topology information extracted from the given topology
    as a dictionary. Keys are the devices mac addresses and the values
    are tuples (is_meshed, mesh_role). is_meshed is a boolean and mesh_role
    is a string typical with the values "master", "slave" or "unknown".
    In case no topology information is available an empty dictionary gets
    returned.
    """
    if topology is None:
        try:
            topology = self.get_mesh_topology()
        except FritzActionError:
            return {}
    return {
        node["device_mac_address"]: (node["is_meshed"], node["mesh_role"])
        for node in topology["nodes"]
    }

The returned mesh_role is as is and it is up to the user to use this information according to the needs. So the method is not "too clever". Also the FritzActionError in case of missing topology information gets handled.

This also enables the user to call get_mesh_topology_info on a FritzHosts instance. In contrast to the _get_mesh_by_mac function which is private to the module.

Furthermore this method is testable. So some tests should be added. That may seem not necessary at first, because it is a very simple method. But for later refactoring it is a safety net.

This dictionary can used in a convenient way by get_hosts_info:

def get_hosts_info(self):
    """
    Returns a list of dicts with information about the known hosts.
    The dict-keys are: 'ip', 'name', 'mac', 'status', 'is_meshed', 'mesh_role'
    """
    result = []
    topology_info = self.get_mesh_topology_info()
    for index in itertools.count():
        try:
            host = self.get_generic_host_entry(index)
        except IndexError:
            # no more host entries:
            break
        mac = host["NewMACAddress"]
        is_meshed, mesh_role = topology_info.get(mac, (None, None))
        result.append(
            {
                "ip": host["NewIPAddress"],
                "name": host["NewHostName"],
                "mac": mac,
                "status": host["NewActive"],
                "is_meshed": is_meshed,
                "mesh_role": mesh_role
            }
        )
    return result

is_meshed and mesh_role are set to None in case of missing data. That's a useful default and again it is up to the caller to handle this.

[AaronDavidSchneider]

Thank you @kbr for your review and for all your work in providing fritzconnection!

I totally agree with your design suggestions and changed it to your suggestions.

Regarding the tests: I didn't find any tests of fritzhosts.py (I hope that's correct). Could you hint me towards how you would like this function to be tested?

Thank you!

[kbr]

@AaronDavidSchneider well, there is no test_fritzhosts.py module so far ;) That's because the get_mesh_topology_info is the first method from fritzhosts.py that can get tested without mocking too much.

If you like to provide the test code and need some help, don't hesitate to ask. Otherwise I'm fine to prepare some tests.

[chemelli74]

if topology is None:
    try:
        topology = self.get_mesh_topology()
    except FritzActionError:
        return {}

I think this way you never update topology when a new device is added to the network.

is_meshed and mesh_role are set to None in case of missing data. That's a useful default and again it is up to the caller to handle this.

So you prefer to have is_meshed as bool | None instead of only bool ?

Simone

[kbr]

I think this way you never update topology when a new device is added to the network.

Nothing has changed but the datastructure and the according lookup. If you don't provide the topology as an argument the topology gets called from the box.

So you prefer to have is_meshed as bool | None instead of only bool ?

Exactly. You get None if there are no data – even not a boolean.

[AaronDavidSchneider]

@AaronDavidSchneider well, there is no test_fritzhosts.py module so far ;) That's because the get_mesh_topology_info is the first method from fritzhosts.py that can get tested without mocking too much.

If you like to provide the test code and need some help, don't hesitate to ask. Otherwise I'm fine to prepare some tests.

Thanks for the info. I think I can try to add some tests. Thanks!

[AaronDavidSchneider]

Added a very simple test @kbr. Actually not sure if its too trivial :D

[AaronDavidSchneider]

I have a question regarding Mac adresses @kbr: When I run get_hosts_info() I will get mesh_role and is_meshed as None and None for my repeater (FRITZ!Repeater 1200). I think this has to do with the different network adapters of the repeater? Do you know if there is any possibility that could help to match the keys?

Edit: Problem solved. See comment below

[kbr]

I don't know and have to merge first to reproduce this. Btw. for new PRs I have created a development branch for not messing all the time with master.

[AaronDavidSchneider]

That makes sense. I just changed the branch.

[AaronDavidSchneider]

So I solved the problem. The issue is that a repeater has multiple mac addresses (for each network adapter one). With the new commit we broadcast the mesh info to every interface. This way it is correctly matched in get_hosts_info().

[kbr]

So I solved the problem. The issue is that a repeater has multiple mac addresses (for each network adapter one). With the new commit we broadcast the mesh info to every interface. This way it is correctly matched in get_hosts_info().

I thought it would be something like this. Thank you :)

[chemelli74]

So I solved the problem.

We ;-)

Simone

[AaronDavidSchneider]

So I solved the problem.

We ;-)

Simone

Sorry :D Not on purpose :D

[kbr]

After merging and testing I have to dive into this topic again:

When I run get_hosts_info() I will get mesh_role and is_meshed as None and None for my repeater (FRITZ!Repeater 1200).

That's indeed strange. But I suppose that

The issue is that a repeater has multiple mac addresses (for each network adapter one). With the new commit we broadcast the mesh info to every interface. This way it is correctly matched in get_hosts_info().

does not address the problem, even if it seems to solve it. I think it is an unreliable side effect and kind of guessing.

Inspecting the topology it turns out to be a list of nodes, representing the devices with the device_mac_addresses and the mesh information. The node_interfaces are representing the edges to build a device graph.

Calling self.get_generic_host_entry will return a list of active and inactive devices, including inactive ones no longer listed in the topology (physical absence or some other reason). In this case there will be None, None for is_meshed and mesh_role.

So broadcasting should not make it into master until it is better understood, why the repeater does not report the proper informations.

[chemelli74]

Inspecting the topology it turns out to be a list of nodes, representing the devices with the device_mac_addresses and the mesh information. The node_interfaces are representing the edges to build a device graph.

Isn't device_mac_address the MAC of the interface used to establish the mesh connection, while under nodes are listed all the MAC addresses of the repeaters (one per interface) ?

Simone

[AaronDavidSchneider]

So broadcasting should not make it into master until it is better understood, why the repeater does not report the proper informations.

Hi, what are we missing for this PR? And how can we get it merged @kbr?

[chemelli74]

Hi @kbr, can we discuss together with @AaronDavidSchneider how to progress on this ?

Thx in advance,

Simone

[chemelli74]

I played a little bit with Fritz topology, and came up with something that I judge interesting.

Do you mind testing my script in your environment ? Link: https://pastebin.com/YjnKf7wa

Example of the output:

Got mesh topology from device
******************
Mesh node: Fritz123
Int. name: LAN:1
Int. mac:  11:22:33:44:55:66
Int. type: LAN
Devices:
  - Dev:    MyNAS
******************
Mesh node: Fritz123
Int. name: LAN:2
Int. mac:  11:22:33:44:55:66
Int. type: LAN
Devices:
  - Dev:    ipcam1
******************
Mesh node: Fritz123
Int. name: AP:5G:0
Int. SSID: <My-Wi-Fi-SSID>
Int. mac:  11:22:33:44:55:66
Int. type: WLAN
Devices:
  - Dev:    Phone1
  - Dev:    Phone2
  - Dev:    Samsung-Salotto-Soundbar
  - Dev:    Samsung-Salotto-TV
  - Dev:    Alexa-Salotto
  - Dev:    Alexa-Camera-da-letto
  - Dev:    fritz.repeater
****************** 

We can enrich host info with a lot of info:

• ssid
• connection type ( lan / wlan )
• parent connection ( mesh node )

Simone

[kbr]

I get a similar output with my configuration. The topology-information structure is not documented, so there must be some reverse engineering to understand whats going on. From my point of view this is a graph representation that can be used to create nice pictures like the one in "Home Network > Mesh > Mesh Overview" at the web-based backend.

[chemelli74]

The topology-information structure is not documented, so there must be some reverse engineering to understand whats going on.

Looking at the json reply to get_mesh_topology():

• topology has a list of nodes.
• each node has a list of interfaces.
• each interface list all connected nodes.

Seems quite simple and linear to me. What you think is missing ? We really want to progress with that so to include more info for each device on HomeAssistant.

Simone

[kbr]

After reading the thread again, I'm unsure about losing the focus: you want to provide more info for each device on HomeAssistant and a call to get_mesh_topology() seems to return this information. So why do you think you can not make any progress? Is there any reason for not taking these informations and provide them in a proper way for HomeAssistant? Am I missing something?

[chemelli74]

Hi, we finally implemented the mesh topology logic in HomeAssistant.

Simone

[kbr]

Well, congratulations – good news for the new year :)

A couple of days ago I also got to this topic again, finding the according documentation by AVM as references to some json files: https://avm.de/fileadmin/user_upload/Global/Service/Schnittstellen/hostsSCPD.pdf (see Table 17)

So there is no need for reverse engineering, the structure is as you mentioned with some additional information.

All the best for your HA project.