fritzconnection icon indicating copy to clipboard operation
fritzconnection copied to clipboard

Add WoL CLI with convenience features

Open chrisbraucker opened this issue 2 years ago • 7 comments

As discussed in #148 and preliminarily implemented in #149, I adapted the code from @samba2 according to requested code style changes from @kbr, then refactored and added additional logic to make it easy to wake a host by either MAC, IP, hostname or the ID number of the internal hosts table of the fritz box.

I'm eager to hear your thoughts on this.

Should close #148 and #149

chrisbraucker avatar Sep 11 '23 14:09 chrisbraucker

umm, you only commented on the PR, not a particular line/file, so I can only guess 🤔

chrisbraucker avatar Sep 11 '23 18:09 chrisbraucker

Thank you very much for the pull-request and sorry for the delay. I have not checked the code by a test run so far, but by style. For the argument-parser I suggest to define separate options instead of a single complex one. I skipped the index n, because the index may change. Also the name could be ambigious, but I kept it because it can be convenient if names are unambigious. Furthermore I would suggest to not define an additional Exception but use the FritzArgumentError Exception, because the errors are argument errors. Regarding this wake_host and add_arguments could change like (untested):

def wake_host(fh, args):

    if args.device_mac_address:
        mac = args.device_mac_address

    elif args.device_ip_address:
        try:
            host = fh.get_specific_host_entry_by_ip(args.device_ip_address)
        except (FritzArgumentError, FritzLookUpError) as err:
            msg = f"Error: unknown IP {args.device_ip_address}"
            raise FritzArgumentError(msg)
        mac = host['NewMACAddress']
        
    elif args.device_name:
        device_name = args.device_name.lower()
        for entry in fh.get_generic_host_entries():
            if entry['NewHostName'].lower() == device_name:
                mac = entry['NewMACAddress']
                break
        else:
            msg = f"Error: unknown device name '{args.device_name}'"
            raise FritzArgumentError(msg)
            
    else:
        msg = f"Error: missing device specification"
        raise FritzArgumentError(msg)
        
    fh.wakeonlan_host(mac)
        

def add_arguments(parser):
    parser.add_argument(
        "--device-ip",
        dest="device_ip_address",
        default=None,
        help="ip-address of device to wake up",
    )
    parser.add_argument(
        "--device-mac",
        dest="device_mac_address",
        default=None,
        help="mac-address of device to wake up",
    )
    parser.add_argument(
        "--device-name",
        dest="device_name",
        default=None,
        help="name of device to wake up",
    )

kbr avatar Jul 16 '24 09:07 kbr

Thank you for the detailed feedback, I'll adapt the code towards your suggestions as soon as my schedule allows for :)

chrisbraucker avatar Jul 17 '24 11:07 chrisbraucker