Re: [chirp_devel] TYT TH-UVF8D support

This looks like a repeat of your mail from 10/31. Is this patch different? Which one should I look at?

--Dan

On 11/05/2013 12:18 PM, Eric Allen wrote:

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 mailto: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 <http://www.danplanet.com>
KK7DS


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

---

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