Cool, thanks! Just a few comments from a quick skim:
> -        return "Memory %i: %s%s%s %s (%s) r%.1f%s c%.1f%s d%03i%s%s [%.2f]"% \
> +        return "Memory %s: %s%s%s %s (%s) r%.1f%s c%.1f%s d%03i%s%s [%.2f]"% \
The number is an integer, so this doesn't make sense, AFAICT.

aw shucks, you're totally right. I had it in there before I figured out how to use special channels correctly.
 
> +    #if data != radio._mmap[:32]:
> +        #raise errors.RadioError("Model mis-match: \n%s\n%s" % (util.hexprint(data),
> +                                                               #util.hexprint(radio._mmap[:32])))
Why is this commented-out?

Thanks for catching that! Disabled it while getting the hang of the memory layout and forgot to re-enable
 
> +
> +    for i in range(0, 0x4000, 0x20):
> +        addr = i + 0x20
> +        msg = struct.pack(">cHb", "W", i, 0x20)
> +        msg += radio._mmap[addr:addr+0x20]
Need space around the + operator here
 
Fixed.

 
> +    def _decode_tone(self, toneval):
> +        pol = "N"
> +        rawval = (toneval[1].get_bits(0xFF) << 8) | toneval[0].get_bits(0xFF)
Doesn't this just mean you need to use the other endianess of your toneval?

I copy-pasted from thuv1f.py without thinking too hard about it. There's a bunch more code in _decode_tone and _encode_tone that rely on this particular endianness. Can we just leave it as-is, since the other code does it this way?
 
> +        if isinstance(number, int):
> +            enabled = self._memobj.enable[(number - 1) / 8].flags[7-((number - 1) % 8)]
> +            dont_skip = self._memobj.skip[(number - 1) / 8].flags[7-((number - 1) % 8)]
Need space around the - operator.

fixed
 
> +        if dont_skip:
> +          mem.skip = ""
> +        else:
> +          mem.skip = "S"
This should be four-space indented.

fixed
 
Also, this could be compacted to:
  mem.skip = "S" if not dont_skip else ""

Thanks! fixed
 
> +        if mem.empty:
> +            _mem.set_raw("\xFF" * 32)
> +            self._memobj.enable[(mem.number - 1) / 8].flags[7-((mem.number - 1) % 8)] = False
> +            self._memobj.skip[(mem.number - 1) / 8].flags[7-((mem.number - 1) % 8)] = False
> +            return
> +        else:
> +            self._memobj.enable[(mem.number - 1) / 8].flags[7-((mem.number - 1) % 8)] = True
Need spaces around the - operator.
fixed
 
> +        self._memobj.skip[(mem.number - 1) / 8].flags[7-((mem.number - 1) % 8)] = (mem.skip == "")
Here too.

fixed


On Mon, Oct 7, 2013 at 8:00 AM, Dan Smith <dsmith@danplanet.com> wrote:
> I acquired a TYT TH-UVF8D several months back, and I've finally found
> the time to finish off adding support for it to CHIRP. In adding support
> for editing the VFO channels as "Special Channels", I had to make a
> small tweak to the command-line utility so it didn't choke on
> non-numeric channel names. Attached is the patch for that, the patch for
> the radio, and an image file dumped from the radio for testing purposes.

Cool, thanks! Just a few comments from a quick skim:

> -        return "Memory %i: %s%s%s %s (%s) r%.1f%s c%.1f%s d%03i%s%s [%.2f]"% \
> +        return "Memory %s: %s%s%s %s (%s) r%.1f%s c%.1f%s d%03i%s%s [%.2f]"% \

The number is an integer, so this doesn't make sense, AFAICT.

> +    #if data != radio._mmap[:32]:
> +        #raise errors.RadioError("Model mis-match: \n%s\n%s" % (util.hexprint(data),
> +                                                               #util.hexprint(radio._mmap[:32])))

Why is this commented-out?

> +
> +    for i in range(0, 0x4000, 0x20):
> +        addr = i + 0x20
> +        msg = struct.pack(">cHb", "W", i, 0x20)
> +        msg += radio._mmap[addr:addr+0x20]

Need space around the + operator here

> +    def _decode_tone(self, toneval):
> +        pol = "N"
> +        rawval = (toneval[1].get_bits(0xFF) << 8) | toneval[0].get_bits(0xFF)

Doesn't this just mean you need to use the other endianess of your toneval?

> +        if isinstance(number, int):
> +            enabled = self._memobj.enable[(number - 1) / 8].flags[7-((number - 1) % 8)]
> +            dont_skip = self._memobj.skip[(number - 1) / 8].flags[7-((number - 1) % 8)]

Need space around the - operator.

> +        if dont_skip:
> +          mem.skip = ""
> +        else:
> +          mem.skip = "S"

This should be four-space indented.

Also, this could be compacted to:

  mem.skip = "S" if not dont_skip else ""

> +        if mem.empty:
> +            _mem.set_raw("\xFF" * 32)
> +            self._memobj.enable[(mem.number - 1) / 8].flags[7-((mem.number - 1) % 8)] = False
> +            self._memobj.skip[(mem.number - 1) / 8].flags[7-((mem.number - 1) % 8)] = False
> +            return
> +        else:
> +            self._memobj.enable[(mem.number - 1) / 8].flags[7-((mem.number - 1) % 8)] = True

Need spaces around the - operator.

> +        self._memobj.skip[(mem.number - 1) / 8].flags[7-((mem.number - 1) % 8)] = (mem.skip == "")

Here too.

--
Dan Smith
www.danplanet.com
KK7DS


_______________________________________________
chirp_devel mailing list
chirp_devel@intrepid.danplanet.com
http://intrepid.danplanet.com/mailman/listinfo/chirp_devel
Developer docs: http://chirp.danplanet.com/projects/chirp/wiki/Developers