Structure code and bug fix

[luismeruje]

Separated code into functions.
Added configuration file to enable/disable crc check, which does not work for all devices (left it disabled by default).
The program now exhausts all reads before exiting the program when a Keyboard Interrupt occurs.
This changes help with issues #12 and #7.

[baryluk]

Thanks for fixing the issue. It is pretty obvious now what was going on with the FIFO blocking the logger. The separation into functions and cleanup of some old code, also make sense.

I do have one request tho.

Instead of config parsers and config file, could you just make it use argparse and command line flags, at the start of the def main():

I really do not like having config files, or extra dependencies (no matter how small).

Something like this:

def main():
    ...
    import argparse

    parser = argparse.ArgumentParser(formatter_class=argparse.ArgumentDefaultsHelpFormatter)
    parser.add_argument("--crc", type=bool, help="Enable CRC checks", default=False)
    parser.add_argument("--dev", type=str, help="....", default="")
    args = parser.parse_args()
    ...
    crc_checker = None  # alternatively lambda x: True
    if args.crc:
        import crc
        crc_checker = lambda_or_something

    # use  args.dev to find specific device (i.e. by USB path/bus/device id)

You can make --crc default to False, or auto-detect by trying to import (I think we might be already doing this).

For booleans on command line I usually use a helper like this:

def str2bool(v: Union[str, bool], default_on_error: bool) -> bool:
    if isinstance(v, bool):
        return v
    vl = v.lower()
    if vl in ("yes", "true", "t", "1"):
        return True
    if vl in ("no", "false", "f", "0"):
        return False
    # raise ValueError("str2bool argument type must be true or false, found {v}")
    return default_on_error

Then in argparse, I use it like this:

    parser.add_argument("--crc", type=str2bool, help="Enable CRC checks", default=True)

Then can do --crc=false, --crc=0, etc

[luismeruje]

@baryluk I understand your concerns, I have substituted configparser for argparse.

[luismeruje]

Also, I suspect that for FNB58 the HID interface, even though it works (more or less), might not be the correct one. The set of interfaces on FNB58 is as follows:

    INTERFACE 0: Mass Storage ==============================
     bLength            :    0x9 (9 bytes)
     bDescriptorType    :    0x4 Interface
     bInterfaceNumber   :    0x0
     bAlternateSetting  :    0x0
     bNumEndpoints      :    0x2
     bInterfaceClass    :    0x8 Mass Storage
     bInterfaceSubClass :    0x6
     bInterfaceProtocol :   0x50
     iInterface         :    0x0 
      ENDPOINT 0x84: Bulk IN ===============================
       bLength          :    0x7 (7 bytes)
       bDescriptorType  :    0x5 Endpoint
       bEndpointAddress :   0x84 IN
       bmAttributes     :    0x2 Bulk
       wMaxPacketSize   :   0x40 (64 bytes)
       bInterval        :    0x0
      ENDPOINT 0x4: Bulk OUT ===============================
       bLength          :    0x7 (7 bytes)
       bDescriptorType  :    0x5 Endpoint
       bEndpointAddress :    0x4 OUT
       bmAttributes     :    0x2 Bulk
       wMaxPacketSize   :   0x40 (64 bytes)
       bInterval        :    0x0
    INTERFACE 1: CDC Communication =========================
     bLength            :    0x9 (9 bytes)
     bDescriptorType    :    0x4 Interface
     bInterfaceNumber   :    0x1
     bAlternateSetting  :    0x0
     bNumEndpoints      :    0x1
     bInterfaceClass    :    0x2 CDC Communication
     bInterfaceSubClass :    0x2
     bInterfaceProtocol :    0x1
     iInterface         :    0x0 
      ENDPOINT 0x82: Interrupt IN ==========================
       bLength          :    0x7 (7 bytes)
       bDescriptorType  :    0x5 Endpoint
       bEndpointAddress :   0x82 IN
       bmAttributes     :    0x3 Interrupt
       wMaxPacketSize   :   0x40 (64 bytes)
       bInterval        :    0xa
    INTERFACE 2: CDC Data ==================================
     bLength            :    0x9 (9 bytes)
     bDescriptorType    :    0x4 Interface
     bInterfaceNumber   :    0x2
     bAlternateSetting  :    0x0
     bNumEndpoints      :    0x2
     bInterfaceClass    :    0xa CDC Data
     bInterfaceSubClass :    0x0
     bInterfaceProtocol :    0x0
     iInterface         :    0x0 
      ENDPOINT 0x81: Bulk IN ===============================
       bLength          :    0x7 (7 bytes)
       bDescriptorType  :    0x5 Endpoint
       bEndpointAddress :   0x81 IN
       bmAttributes     :    0x2 Bulk
       wMaxPacketSize   :   0x40 (64 bytes)
       bInterval        :    0x0
      ENDPOINT 0x1: Bulk OUT ===============================
       bLength          :    0x7 (7 bytes)
       bDescriptorType  :    0x5 Endpoint
       bEndpointAddress :    0x1 OUT
       bmAttributes     :    0x2 Bulk
       wMaxPacketSize   :   0x40 (64 bytes)
       bInterval        :    0x0
    INTERFACE 3: Human Interface Device ====================
     bLength            :    0x9 (9 bytes)
     bDescriptorType    :    0x4 Interface
     bInterfaceNumber   :    0x3
     bAlternateSetting  :    0x0
     bNumEndpoints      :    0x2
     bInterfaceClass    :    0x3 Human Interface Device
     bInterfaceSubClass :    0x0
     bInterfaceProtocol :    0x0
     iInterface         :    0x0 
      ENDPOINT 0x83: Interrupt IN ==========================
       bLength          :    0x7 (7 bytes)
       bDescriptorType  :    0x5 Endpoint
       bEndpointAddress :   0x83 IN
       bmAttributes     :    0x3 Interrupt
       wMaxPacketSize   :   0x40 (64 bytes)
       bInterval        :    0xa
      ENDPOINT 0x3: Interrupt OUT ==========================
       bLength          :    0x7 (7 bytes)
       bDescriptorType  :    0x5 Endpoint
       bEndpointAddress :    0x3 OUT
       bmAttributes     :    0x3 Interrupt
       wMaxPacketSize   :   0x40 (64 bytes)
       bInterval        :    0xa

I suspect Interface 2 might be the correct one, and hence why CRC does not work for FNB58. However, simply swapping the interfaces is not sufficient to get it to work and I do not know what commands to send to the device for that interface.