kbr / fritzconnection

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

check is key (NewX_AVM-DE_PossibleBeaconTypes) available #191

Closed Ludy87 closed 11 months ago

Ludy87 commented 11 months ago
  File "/home/vscode/.local/lib/python3.11/site-packages/fritzconnection/lib/fritzwlan.py", line 107, in get_wifi_qr_code
    security = get_beacon_security(instance, security)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/lib/python3.11/site-packages/fritzconnection/lib/fritzwlan.py", line 47, in get_beacon_security
    beacontypes = set(info["NewX_AVM-DE_PossibleBeaconTypes"].split(","))
                      ~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
KeyError: 'NewX_AVM-DE_PossibleBeaconTypes'
Ludy87 commented 11 months ago

Modell:FRITZ!Box WLAN 3370 FRITZ!OS: 06.55

Feel free to change the code. 😉

Ludy87 commented 11 months ago

I made a few changes to use the qr code in Homeassistant as well

kbr commented 11 months ago

I also have to admit, that the changes from the third diff 9b8430d will not work, because the return value from the function get_beacon_security() is used to get forwarded to the segno-library which only accepts None, "nopass", "WEP" or "WPA" as values for the security-argument.

kbr commented 11 months ago

Your original intention was to solve the KeyError: 'NewX_AVM-DE_PossibleBeaconTypes'. This is something worth a merging according to the 643bcec changeset with the mentioned additional line of code.

kbr commented 11 months ago

Sorry, seems I have messed up a bit with the GitHub interface. I was referring to this change for merging: https://github.com/kbr/fritzconnection/pull/191#discussion_r1280696517

Ludy87 commented 11 months ago

I've undone a lot, sorry for changing your style - black had changed it and I disregarded it.

kbr commented 11 months ago

Thanks – yes, black can save the day or mess up the diff on existing projects.

If I get it right, the changes are now reduced to give this version:

def get_beacon_security(instance, security):
    "docstring here ..."
    if not security:
        info = instance.get_info()
        beacontype = info["NewBeaconType"]
        # check for the POSSIBLE_BEACON_TYPES_KEY argument
        # as older models may not provide it:
        if POSSIBLE_BEACON_TYPES_KEY in info:
            beacontypes = set(info[POSSIBLE_BEACON_TYPES_KEY].split(","))
            beacontypes -= set(('None', 'OWETrans'))
            if beacontype in beacontypes:
                security = WPA_SECURITY
        elif beacontype:
            if beacontype == NO_PASS:
                security = NO_PASS
            else:
                security = WPA_SECURITY
    return security

There ist still the problem that the beacontype in case of no encryption is not returned as None but as "None". Also in case of a modern router but no encryption security will get returned with the provided value but not as NO_PASS. Cleaning up the program flow a little bit, this change can be included like:

def get_beacon_security(instance, security):
    "docstring here ..."
    if not security:
        security = NO_PASS
        info = instance.get_info()
        beacontype = info["NewBeaconType"]
        # check for the POSSIBLE_BEACON_TYPES_KEY argument
        # as older models may not provide it:
        if POSSIBLE_BEACON_TYPES_KEY in info:
            beacontypes = set(info[POSSIBLE_BEACON_TYPES_KEY].split(","))
            beacontypes -= set(('None', 'OWETrans'))
            if beacontype in beacontypes:
                security = WPA_SECURITY
        else:
            # dealing with an older model
            # assuming at least providing WPA security
            # (unable to test for WEP because of missing hardware)
            if beacontype != "None":
                security = WPA_SECURITY
    return security

I have not used an elif here in favour for the comment ("readability counts"). Also the code is not tested but I suppose (or hope) there should be no hidden errors any more.

Ludy87 commented 11 months ago

I tested it in my interest and it works.

kbr commented 11 months ago

Thank you for the commits and the patience.

Ludy87 commented 11 months ago

Es war deine Geduld, die uns zum Ziel gebracht hat.