Improvement request: FritzStatus and FritzConnection

[chemelli74]

Hi @kbr,

I would like to add some calls to fritzconnection. In details:

FritzConnection:

- do_update:                call_action("UserInterface:1", "X_AVM-DE_DoUpdate")

FritzStatus:

- get_wanip_info:       call_action("WANIPConn", "GetStatusInfo")
- get_wancommon_addoninfo:  call_action("WANCommonIFC1", "GetAddonInfos")
- get_wandls_info:      call_action("WANDSLInterfaceConfig1", "GetInfo")

- get_all_info:         parse and combine the 3 above

The idea is to get all info limiting the number of call towards the device.

We would also need such approach to get a better implementation on HomeAssistant side so we can easily get a DataUpdateCoordinator to fetch and cache the needed info.

Will you accept related PRs ?

Thx,

Simone

cc @Mib1185

[chemelli74]

Forgot to mention X_AVM-DE_GetHostListPath that will be implemented with more or less the same code as get_mesh_topology

Simone

[kbr]

To provide a method for X_AVM-DE_GetHostListPath makes sense, as it requires some post-processing.

For the other ones: why just not call call_action? That's the way the library works under the hood. So you don't have to wait for PRs to merge.

For i.e. if you have a fritzconnection instance fc, you can implement this easily:

def get_wan_ip_info(fc):
    """Return all wan info from the device
    """
    info = {}
    for service, action in [("WANIPConn", "GetStatusInfo"), 
                            ("WANCommonIFC1", "GetAddonInfos"),
                            ("WANDSLInterfaceConfig1", "GetInfo")]:
        info.update(fc.call_action(service, action))
    return info

[chemelli74]

Of course we can easily go with an implementation like you suggested, but HomeAssistant pushes for all low level calls to happen on the library side. If you could not consider my proposal, no problem. In a way or in another, I hope to be able to get the needed functionality in HomeAssistant ;-)

As we agree on X_AVM-DE_GetHostListPath, could you go for it ? :-)

Thx in advance,

Simone

[kbr]

Regarding low level: everything that happen behind call_action can be considered as low level. But call_action is the API to send a request to the router and get a result already converted to the corresponding python data types. Everything else on top (the library) is just convenience.

HomeAssistant pushes for all low level calls to happen on the library side.

I can follow the basic idea behind this, but it is a restriction that can easily get circumvented: just write an adapter class as library substitute.

As we agree on X_AVM-DE_GetHostListPath, could you go for it ? :-)

May be, but no timeline, because next busy season is on the horizon.

[chemelli74]

As we agree on X_AVM-DE_GetHostListPath, could you go for it ? :-)

May be, but no timeline, because next busy season is on the horizon.

What if I do the coding ? ;-)

Simone

[kbr]

In this case you have to go through the review ;-)

My first idea was to reuse the processor from core.processor, but according to the AVM documentation page 8 ff. this will not work, because some tag names have dashes. So the next idea is to convert the tags more to pep8 and provide them as attributes on item objects. Or keep the tags for use as keys in a dict, to keep in better sync with the AVM doc. Or both :)

[kbr]

Well, I see that it would just take a minor code change in the processor to convert the dashes to underlines, so that the tag-names are useable as attributes (with this minor name change). That would make it very easy to convert the xml-content to a list of item-objects.

[kbr]

@chemelli74: I was curious and implemented a prototype of FritzHosts.get_host_list() in the branch dev/host_list. Unsure about the proper method name, because it returns not a single host and datatypes in labels are no good style. Feel free to play around.

[chemelli74]

@chemelli74: I was curious and implemented a prototype of FritzHosts.get_host_list() in the branch dev/host_list. Unsure about the proper method name, because it returns not a single host and datatypes in labels are no good style. Feel free to play around.

Thx for the quick feedback! Really appreciated!

Today, as I'm back home, I was able to do a first test. I have a concern about a few values:

"X_AVM-DE_UpdateAvailable": true,
"X_AVM-DE_Guest": true,
"X_AVM-DE_VPN": true,

Those are the same for all my devices, and on 99% of the cases are wrong.

Finally there are 2 values contrasting:

"X_AVM-DE_WANAccess": "granted",
"X_AVM-DE_Disallow": true

Disallow should be false

Any idea it there is some issue with conversion or is the API broken ?

Simone

[kbr]

Thank you for testing – I was wondering about this too. As I can see now, it is a bug in the converter dictionary: bool gets not applied to 1 or 0 but to '1' or '0', which always is True. Have commited a quick fix .

[chemelli74]

Have commited a quick fix .

Did a very quick test, values seem fine now. Great job!

Simone

[chemelli74]

After more testing, all seems working as expected.

Can you please consider merging and releasing 1.9.2 ? :-)

[kbr]

As it is a new feature, I suppose this is a candidate for 1.10 and not a hot-fix like 1.9.1 :)

[chemelli74]

Do you have a timeframe in mind for the new major release ? If so, can you share it with us ?

Simone

[kbr]

There is no timeframe like "one update every two weeks" or so. I try to cover hot-fixes (like 1.9.1) at patch-level as soon as possible. Minor releases are typically done when new features have been implemented, tested and documented. In general this project tries to follow semantic versioning for releases. That is also not covered by the documentation so far. But FritzHosts.get_host_list() is on the roadmap :)

[chemelli74]

Klaus, would be super cool to get this merged as well ;-)

Thank you for considering.

Simone

[kbr]

It's committed but with a different implementation and a different name (get_hosts_attributes) because this better describes the result of the processed "X_AVM-DE_GetHostListPath"-action.

[chemelli74]

Thx, didn't noticed!

So I guess we can go as far as closing this issue.

Simone

[kbr]

Well – the commit was just a few hours ago.