kbr / fritzconnection

Python-Tool to communicate with the AVM Fritz!Box by the TR-064 protocol and the AHA-HTTP-Interface
MIT License
304 stars 59 forks source link

Mix of strings and booleans in action results and parameters #30

Closed mammuth closed 4 years ago

mammuth commented 4 years ago

Hi, we're using fritzconnection over at https://github.com/mammuth/ha-fritzbox-tools (a custom component for Home Assistant). Thanks for providing the library (and moving it forward with updates) 🚀

When upgrading from 0.8.4 to 1.2.0 we noticed that argument and return types of action calls don't share the same type anymore.

See the following example:

>>>c.call_action("WLANConfiguration:3", "GetInfo")
# Note: 'NewEnable' in the result dict is a boolean
{'NewEnable': True, 'NewStatus': 'Up', 'NewMaxBitRate': 'Auto', 'NewChannel': 11, 'NewSSID': 'myssid', [...]}

# Let's try setting the state of the guest wifi by passing NewEnable=False (seems correct, since it just returned `False`, doesn't it?
>>> c.call_action("WLANConfiguration:3", "SetEnable", NewEnable=False)
Traceback (most recent call last):
  File "<input>", line 1, in <module>
    c.call_action("WLANConfiguration:3", "SetEnable", NewEnable=False)
  File "/usr/local/lib/python3.7/site-packages/fritzconnection/core/fritzconnection.py", line 209, in call
_action
    return self.soaper.execute(service, action_name, arguments)
  File "/usr/local/lib/python3.7/site-packages/fritzconnection/core/soaper.py", line 170, in execute
    return handle_response(response)
  File "/usr/local/lib/python3.7/site-packages/fritzconnection/core/soaper.py", line 153, in handle_respon
se
    raise_fritzconnection_error(response)
  File "/usr/local/lib/python3.7/site-packages/fritzconnection/core/soaper.py", line 82, in raise_fritzcon
nection_error
    raise exception(message)
fritzconnection.core.exceptions.FritzArgumentError: UPnPError: 
errorCode: 402
errorDescription: Invalid Args

# Using "0" (as in previous versions) works fine
>>> c.call_action("WLANConfiguration:3", "SetEnable", NewEnable="0")
{}

When dealing with port forwards this produces some more overhead because the AddPortMapping action requires the whole dictionary to be passed when you want to turn on/off the port forward. So you would store the response dict of GetGenericPortMappingEntry (which has a boolean) and then have to change it to "[0,1]" when using it as an argument for the AddPortMapping action.

While people using fritzconnection are surely able to handle this, I think it should be streamlined in fritzconnection itself, so it's easier to use. What's your opinion on this?

We also noticed a change in the format of the services dict (FritzConnection(...).services). While it used to be eg. {"WLANConfiguration:3": {...}}, it's now {"WLANConfiguration3": {...}} etc.

kbr commented 4 years ago

This seems to be indeed a candidate for improvement. The Fritz-interface knows about booleans but represents them as 0 and 1. All parameters are sent and received as strings. For convenience the received values are converted to the corresponding Python-datatypes. For sending all arguments are converted to strings, which does not work for True and False. I will put this on the todo-list.

Regarding the other topic: "WLANConfiguration3" is the correct way for naming services (introduced with 0.8.0). However, the old way to do it like "WLANConfiguration:3" is still supported and will not get deprecated.

kbr commented 4 years ago

With version 1.3.0 call_action() accepts True and False for boolean arguments, additionally to '1' and '0'. So I like to close this. Please feel free to reopen in case of trouble.