[chirp_devel] New IC-7300 Driver

[Dan Smith]

> +def _setbaud(radio):
> +    """ Determine fastest valid baud rate """
> +    global BAUD
> +    radio.pipe.timeout = 0.1
> +    for BAUD in BAUDRATES:       # Attempt to read CI-V Transceiver status
> +        radio.pipe.baudrate = BAUD
> +        radio.pipe.write("\xFE\xFE\x94\xE0\x1A\x05\x00\x71\xFD")
> +        resp = radio.pipe.read(55)
> +        if len(resp) > 17:
> +            if resp[17] == "\x01":
> +                break        # exit For loop
> +    if BAUD < 4800:
> +        msg = "Unable to coomunicate with radio! Cable? Power?"
> +        raise errors.RadioError(msg)
> +    LOG.debug("Baud Rate is %d" % BAUD)
> +    radio.pipe.timeout = STIMEOUT
> +    return

I've mentioned this before, but please do not use a global for things like BAUD. It means that talking to two of these radios at the same time, configured for different baudrates will not work. Set a property on the radio instance and use that after you've probed for the specific radio, if necessary. However, once you set the baud rate on radio.pipe, it sticks until you close/re-open the port, so there really should be no need.
> 
> +def dbg_dump(stz, kn=32, cptn="DBG", ax=False, lgx=1, ndx=False):

Why not utils.hexprint()? No need to re-implement that here.

> +def _rhcd_decode(stx, nbyt, lgw=False):
> +    """ Decode decimal value from reverse-hex stx backwards starting
> +        at nbyt index. 4 for freq, 3 for offset.
> +        Returns 4-byte UL32 string. Ignore upper 0's if format is less"""
> +    vstr = ""
> +    for ix in range(nbyt-1, -1, -1):
> +        vstr += "%02x" % ord(stx[ix])
> +    vx = int(vstr)
> +    if lgw:
> +        LOG.warning("vstr: %s, vs: %04x" % (vstr, vx))
> +    vstr = chr(vx & 255)
> +    vx = vx >> 8
> +    vstr += chr(vx & 255)
> +    vx = vx >> 8
> +    vstr += chr(vx & 255)
> +    vx = vx >> 8
> +    vstr += chr(vx & 255)
> +    return vstr

Why not use bitwise for this? It's this just lbcd?

> +
> +
> +def _fhdc_decode(stx, nbyt, lgw=False):
> +    """ Decode decimal value from hex string stx, not reversed.
> +        nbyt is length """
> +    vstr = ""
> +    for ix in range(0, nbyt):      # Decode hcd rcts
> +        vstr += "%02x" % ord(stx[ix])
> +    vx = int(vstr)
> +    if lgw:     # print values
> +        LOG.warning("fhdc vstr: %s, vx: %i" % (vstr, vx))
> +    return vx

...

> +def _read_mem(radio):
> +    """Generate the memory map. """
> +    radio.pipe.baudrate = BAUD
> +    # Flush the input buffer
> +    radio.pipe.timeout = 0.005
> +    junk = radio.pipe.read(256)

At slow baud rates, and on loaded systems, such a short timeout is likely to not accomplish the goal and thus be brittle, especially across operating systems and serial drivers. Why are you even flushing the input buffer on each read anyway? Is the radio sending something asynchronously? If so, we should read and parse it, even if we throw it away.

> + at directory.register
> +class IC7300Radio(chirp_common.CloneModeRadio):

This isn't really a clone-mode radio anyway, right? All the other Icom CI-V radios inherit from the base class that makes them suitably live. I don't think it makes any sense to have this one be totally different. The other similar icom radio drivers are a few lines of code in icomciv.py and I doubt this one is suitably different.

I'm going to stop reviewing here for the moment, as I think that's the right approach, so let me know if there is some really compelling reason not to do that.

--Dan