Device has WAN enabled

[chemelli74]

Add a method to know if the device is configured as a router or not. Needs #146

cc @mib1185

[kbr]

Like #146 this should fit better at application level or in the library, because it is about state interpretation (again, FritzStatus may be the proper place?). Also this code breaks on Python < 3.8 and I find the name a bit misleading, because there can be more routers in the network but not configured as the WAN access point.

[chemelli74]

Like #146 this should fit better at application level or in the library, because it is about state interpretation (again, FritzStatus may be the proper place?).

Agree, will move it there.

Also this code breaks on Python < 3.8

As it seems we agree on adding it, I will write test and rework where needed.

and I find the name a bit misleading, because there can be more routers in the network but not configured as the WAN access point.

What about "device_has_routing_enable" ?

Simone

[chemelli74]

@kbr can you kindly confirm the name I suggested or suggest a new one so I can update the PR ?

Thank you in advance,

Simone

[kbr]

Well, finding good names can be hard. I would suggest has_wan_access because it is about a property of the device and returns a boolean. But I'm also open for a better name.

[chemelli74]

Well, finding good names can be hard. I would suggest has_wan_access because it is about a property of the device and returns a boolean. But I'm also open for a better name.

wan_enabled or has_wan_enabled? I would avoid has_wan_access because the device can access WAN but not using it's own WAN.

Simone

[kbr]

Hi Simone, good point. I would prefer the shorter version. Most likely it will get used as

if device_status.wan_enabled: 
    # do stuff here ...

what is short, readable and transports the meaning: this device is the router in the network which is the active WAN access point.

+1

[chemelli74]

@kbr do you think we are ok for a merge ?

Simone

[chemelli74]

Fixed conflict

[chemelli74]

Can we kindly progress with this PR please ?

Thx in advance,

Simone

[kbr]

With the supposed changes in #146 this may further simplify to (untested):

@property
def has_wan_enabled(self):
    """
    True if wan is enabled otherwise False.
    """
    return self.fc.call_action(self.connection_service, "GetInfo")["NewEnable"]

[chemelli74]

With the supposed changes in #146 this may further simplify to (untested):

One #146 is merged, I'll update this as well.

Thx,

Simone

[chemelli74]

Updated, even if I would have preferred to return False instead of raising an error in case the device has no WAN service.

Simone

[kbr]

I'm a bit confused: returning a boolean is ok, at least for the has_wan_enabled property. I've just added the connection_service property to make the code a bit more convenient. This way all is left should be to add

@property
def has_wan_enabled(self):
    """
    True if wan is enabled otherwise False.
    """
    return self.fc.call_action(self.connection_service, "GetInfo")["NewEnable"]

Or do I miss something?

[chemelli74]

With current PR code, call_action can fail when the device has no WAN service. This is why I would prefer to guard against this situation and return None or False

[kbr]

Understand. It's not about WAN service disabled, but about a device with no WAN service at all. Didn't have had this in mind and was on the wrong track – will think about this again.

[chemelli74]

BTW, this can happen also for just merged "get_default_connection_service"

[kbr]

Yes. But the philosophy is: unknown service -> raise ServiceError. I don't want to see code breaking this. has_wan_enabled makes no sense on a device not supporting wan-access. Therefore raising an exception in this case is ok for me. But an idea could be a property like has_wan_service or has_wan_access (or some other meaningful name) for returning a boolean.

[chemelli74]

Great, then I'll add the new property and then use it to guard this PR.

Fine for you?

If so, can I add it here or prefer a separate PR?

Thanks a lot for working with me on this.

Simone

[kbr]

A new pull request would be fine. And I'm still thinking about a proper name which makes it clear, that it is not about an inactive, but a provided or not provided service ... like the pitfall for me ;)

[chemelli74]

A new pull request would be fine. And I'm still thinking about a proper name which makes it clear, that it is not about an inactive, but a provided or not provided service ... like the pitfall for me ;)

Please check #162