[chirp_devel] [patch] initial support for AnyTone 778uv

[Dan Smith]

Hi Joe,

> Patch and factory image attached.  It's my first chirp driver, so
> I'm sure there will be things to change.  Suggestions welcome.

Sorry for the delay. Lockdown hasn't been less busy for me, unfortunately so I have been woefully behind on processing this and Rick's updated patch. Thanks for working on this, and your patience. I haven't made it through this all the way, but will give you some early feedback on what I've seen so far.

> +struct {
> +  bbcd freq[4];
> +  bbcd offset[4];
> +  u8 unknown1;
> +  bit flags1[8];

Wow, I totally forgot about this bit type. I should be using this more. However, I think you don't actually want to use this. This is for arrays of homogenous flag bits, where we're mapping memory number into a bitfield of presence bits or something. Since this appears to be an array of heterogenous flags, I think you'd be better doing something like this:

u8 talkaround:1,
   scramble:1,
   unknown:2,
   txpower:2,
   negsplit:1,
   possplit:1;

and then you can refer to them by bit:

if _mem.talkaround:
   ...

which removes the need to do all your bit-shifting math.

> +def reprdata(bindata):
> +    hexwidth = 60
> +    sepwidth = 4
> +
> +    line = ''
> +    sepctr = 0
> +    for b in bytes(bindata):
> +        line += '%02x' % ord(b)
> +        sepctr += 1
> +        if sepctr == sepwidth:
> +            line += ' '
> +            sepctr = 0
> +
> +    line += ' ' * (hexwidth - len(line))
> +
> +    for b in bindata:
> +        if 0x21 <= ord(b) <= 0x7E:
> +            line += b
> +        else:
> +            line += '.'
> +    return line

I think utils.hexprint() is what you want. It'll print hex/ascii which I hope would be enough.

> +# Download data from the radio and populate the memory map
> +def do_download(radio):
> +    '''Download memories from the radio'''
> +
> +    # Get the serial port connection
> +    serial = radio.pipe
> +
> +    try:
> +        enter_program_mode(radio)
> +
> +        # now read the contents of all 200 memories
> +        # TODO there's a load of other stuff we could read here
> +        memory_data = ''
> +
> +        # read the occupied memories
> +        # TODO use this to guide read memories
> +
> +        for addr in ADDR_OCCUPIED_MEMORIES + ADDR_SCAN_MEMORIES:
> +            read_command = struct.pack('>BHB', 0x52, addr, MEMORY_RW_BLOCK_SIZE)
> +            read_response = send_serial_command(serial, read_command,
> +                                                MEMORY_RW_BLOCK_CMD_SIZE)
> +            LOG.debug('read response: %s' % reprdata(read_response))
> +
> +            # TODO: check checksums
> +            address, data, valid = parse_read_response(read_response)
> +            memory_data += data
> +
> +        # status info for the UI
> +        status = chirp_common.Status()
> +        status.cur = 0
> +        status.max = (MEMORY_ADDRESS_RANGE[1] -
> +                    MEMORY_ADDRESS_RANGE[0])/MEMORY_RW_BLOCK_SIZE
> +        status.msg = 'Cloning from radio...'
> +        radio.status_fn(status)
> +
> +        # TODO the vendor s/w does some magic here and only reads the occupied
> +        # memories
> +        for read_address in range(MEMORY_ADDRESS_RANGE[0],
> +                                  MEMORY_ADDRESS_RANGE[1] +
> +                                  MEMORY_RW_BLOCK_SIZE, MEMORY_RW_BLOCK_SIZE):
> +            read_command = struct.pack('>BHB', 0x52, read_address,
> +                                       MEMORY_RW_BLOCK_SIZE)
> +            read_response = send_serial_command(serial, read_command,
> +                                                MEMORY_RW_BLOCK_CMD_SIZE)
> +            LOG.debug('read response: %s' % reprdata(read_response))
> +
> +            # TODO: check checksums
> +            address, data, valid = parse_read_response(read_response)
> +            memory_data += data
> +
> +            # update UI
> +            status.cur = (read_address -
> +                          MEMORY_ADDRESS_RANGE[0])/MEMORY_RW_BLOCK_SIZE
> +            radio.status_fn(status)
> +        exit_program_mode(radio)

Hmm, it looks to me like you're reading the memory out of order, which is fine, but also constructing the image out of order as well. It's cool to try to optimize the read, which looks like future work, but we need to construct a whole and faithful image. That means the image should mirror what is in the radio. You can read the mapping first, but you should construct data to fill the image for the sections you don't read (according to what it looks like in the radio) and make sure to stash the mapping information you read early into the memory map in the right location based on where it is in the radio. Also, make sure you're reading everything else if there is a gap between the end of the memory and those flags. The OEM software has the advantage of knowing what the memory layout of the radio should look like, but since we don't, we should try to capture as much of it faithfully as we can.

> +    except errors.RadioError, e:
> +        raise e

Please use python3-compatible syntax in new code (should be "except exception as e".

> +    except Exception, e:
> +        raise errors.RadioError('Failed to download from radio: %s' % e)
> +
> +    return memmap.MemoryMap(memory_data)

Since this is a new driver, please use the newer format so that this has a hope of being python3 compatible. That means using MemoryMapBytes() here and setting NEEDS_COMPAT_SERIAL=False on your radio class. See the tk8180 driver for one example. (There's not really much way you could have known this, without following the archives here, sorry but thanks).

> +        rp.experimental = \
> +            ('This kinda works for me, see if it works for you'
> +             'and let me know: joe at milbourn.org.uk '
> +             'If you break it, you broke it, and you get '
> +             'to keep the all the bits.')

In the final version we commit, I'd like this just to mirror what some of the other drivers mention. Things you haven't tested, concerns you have, or just a statement that it's experimental. People can file a bug report for issues, which we can assign to you. I'd like to not have direct emails in these just because it reduces visibility for everyone else that there may be a problem.

> +        # TODO radio does support dtcs, but I've not looked into that yet
> +        # so it's disabled here.
> +        rf.has_dtcs = False

This really needs to be implemented in the final version. DTCS is too core and common to not include it.

This is as far as I've gotten thus far, but overall it looks like a really solid first draft, so thanks and kudos!

--Dan