Re: [chirp_devel] TYT TH-UVF8D support
Same message and patch, but I wasn't sure the one from gmail got through due to mailing list configuration. Sorry for the confusion!
On Fri, Nov 15, 2013 at 5:34 PM, Dan Smith dsmith@danplanet.com wrote:
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 hereFixed.
> + 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 finallyfound
> 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
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
Whoops, caught a typo in the settings. Not sure how that crept in there, but here's an updated patch.
On Fri, Nov 15, 2013 at 5:50 PM, Eric Allen eric@hackerengineer.net wrote:
Same message and patch, but I wasn't sure the one from gmail got through due to mailing list configuration. Sorry for the confusion!
On Fri, Nov 15, 2013 at 5:34 PM, Dan Smith dsmith@danplanet.com wrote:
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 hereFixed.
> + 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 finallyfound
> 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 makea
> 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
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
Oh, awesome! Didn't realize it'd already made it in. Here ya go.
On Sun, Nov 17, 2013 at 11:13 AM, Dan Smith dsmith@danplanet.com wrote:
Whoops, caught a typo in the settings. Not sure how that crept in there, but here's an updated patch.
Your patch has already been applied, so I need a patch to fix the typo that is based off the tip of the tree.
--Dan
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
Oh, awesome! Didn't realize it'd already made it in. Here ya go.
Thanks, but it needs a bug number in the commit log or the commit hook will fail. You can re-use the same one as the main patch since it's just cleanup of that.
You can tell that things have been applied by looking at the test report email which shows everything new that's being tested. Also by the "new build" announcement that goes to the -users list.
--Dan
Gotcha. Thanks!
Attached with reference to #1057
On Sun, Nov 17, 2013 at 1:38 PM, Dan Smith dsmith@danplanet.com wrote:
Oh, awesome! Didn't realize it'd already made it in. Here ya go.
Thanks, but it needs a bug number in the commit log or the commit hook will fail. You can re-use the same one as the main patch since it's just cleanup of that.
You can tell that things have been applied by looking at the test report email which shows everything new that's being tested. Also by the "new build" announcement that goes to the -users list.
--Dan
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
participants (3)
-
Dan Smith -
Eric Allen -
Eric Allen