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

Problem access device info #15

Closed Themanwithoutaplan closed 4 years ago

Themanwithoutaplan commented 4 years ago

I can't replicate that error on the 1.0 branch but I do have trouble connecting even with the correct password from the CLI. Testing on a Fritz 6490 Unitymedia.

fritzconnection charlieclark$ fritzconnection -i fritz.box -p pw

FritzConnection v1.0a1
Traceback (most recent call last):
  File "/Users/charlieclark/Projects/fritzconnection/bin/fritzconnection", line 11, in <module>
    load_entry_point('fritzconnection', 'console_scripts', 'fritzconnection')()
  File "/Users/charlieclark/Projects/fritzconnection/fritzconnection/cli/fritzinspection.py", line 139, in main
    inspector.view_header()
  File "/Users/charlieclark/Projects/fritzconnection/fritzconnection/cli/fritzinspection.py", line 35, in view_header
    print(self.fc)
  File "/Users/charlieclark/Projects/fritzconnection/fritzconnection/core/fritzconnection.py", line 77, in __repr__
    return f'{self.device_manager.modelname} at ip {self.soaper.address}'
  File "/Users/charlieclark/Projects/fritzconnection/fritzconnection/core/devices.py", line 33, in modelname
    return self.descriptions[0].device_model_name
IndexError: list index out of range

Moved from #12

Themanwithoutaplan commented 4 years ago

So, this does look like it is related to lxml being able to parse URLs directly, in which case the doc string is wrong as are the tests. As we have requests anyway, we should be able to use to to get the resource before handing it on for parsing.

FritzConnection v1.0a1
Traceback (most recent call last):
  File "/Users/charlieclark/Projects/fritzconnection/bin/fritzconnection", line 11, in <module>
    load_entry_point('fritzconnection', 'console_scripts', 'fritzconnection')()
  File "/Users/charlieclark/Projects/fritzconnection/fritzconnection/cli/fritzinspection.py", line 137, in main
    inspector = FritzInspection(
  File "/Users/charlieclark/Projects/fritzconnection/fritzconnection/cli/fritzinspection.py", line 32, in __init__
    self.fc = FritzConnection(address, port, user, password)
  File "/Users/charlieclark/Projects/fritzconnection/fritzconnection/core/fritzconnection.py", line 73, in __init__
    self.device_manager.load_service_descriptions(address, port)
  File "/Users/charlieclark/Projects/fritzconnection/fritzconnection/core/devices.py", line 60, in load_service_descriptions
    service.load_scpd(address, port)
  File "/Users/charlieclark/Projects/fritzconnection/fritzconnection/core/nodes.py", line 127, in load_scpd
    tree = etree.parse(url)
  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/xml/etree/ElementTree.py", line 1202, in parse
    tree.parse(source, parser)
  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/xml/etree/ElementTree.py", line 584, in parse
    source = open(source, "rb")
FileNotFoundError: [Errno 2] No such file or directory: 'http://fritz.box:49000/any.xml'
Themanwithoutaplan commented 4 years ago

OK, the first exception is triggered even by the repr of the connection object. We should have all falback for this, I think.

Themanwithoutaplan commented 4 years ago

FWIW the way nodes work makes debugging and testing really hard because it's so depedent upon side-effects. I generally find that init s themselves should do nothing that isn't directly testable, ie. encapsulated in a relevant method.

kbr commented 4 years ago

If you have a better idea about the nodes please let me known. I'd thought about alternatives to implementing it this way, but didn't have had a better idea back then.

Instead of rewriting Description.add_description (and the according documentation) I would to keep it this way and suggest to take care that the source parameter is indeed what's documented: the xml-source. So in FritzConnection.__init__ a call to requests should be added; may be not implemented in __init__ itself but as a separate method of the FritzConnection class.

Themanwithoutaplan commented 4 years ago

I think we can get similar behaviour that is easier to test and debug with minor changes, which I think the current one isn't: the errors are not showing up at parse/scan time so are difficult to pin down.

I'd be tempted to use descriptors for the class-specific attributes because they document the API much better and it's not as if you want really, really dynamic classes.

kbr commented 4 years ago

I like your idea how to use requests in the description module. About desciptors I would like to think twice, how the dynamic nature of the description files can be handled better this way. Unfortunately in the next two weeks I don‘t have time to dive into coding. Hopefully I may have some ideas thought.

Themanwithoutaplan commented 4 years ago

No hurry on either front. I think a separate function for calling the box to get the info is the way to go and we can add logging to it for debugging purposes.

kbr commented 4 years ago

That's about the entry point, actually DeviceManager.add_description. I think, that's ok. Nodes are working, but less dynamic would be better as it is really magic the way it is now.

kbr commented 4 years ago

I like to close this because of replacing nodes.py with processor.py 4e4cfb18e9b911d7200da10ec00638f0d13a6556