Overhaul of Kenwood D7 Driver #674

W8BSD · 2023-06-23T06:53:03Z

Removes dependency on kenwood_live, and implements a separate
radio class for D7 series radios.

Moves globals into the classes

This allows finer control with subclassing

Clean up I/O

Don't require a timeout after each command, speeding up access
significantly
Read all "garbage" bytes at the start rather than "just" 25

Add generic support for special channels

Used for scan memories and call memories in the D7s

Makes settings easier to implement

Data-driven description of settings with built-in support for
bools, ints, strings, lists, and maps, which is enough for most
settings
Also supports custom classes for more complex settings, used for
positions, DTMF memories, and programmable VFOs currently
Allows for settings to depend on other settings
Used for automatic simplex check and tone alert settings

Add full menu structure from the manuals.

All settings listed in the manuals that can be controled via
serial port are now exposed in Chirp

W8BSD · 2023-06-23T19:15:06Z

With the bare import of kenwood_live apparently needed to modify LAST_BAUD, I'm seriously considering disconnecting this driver from kenwood_live completely. At this point the amount of shared code is low enough that I'm not even really sure how much there even is left.

W8BSD · 2023-06-24T04:17:24Z

I'm seriously considering disconnecting this driver from kenwood_live completely.

Done.

W8BSD · 2023-06-24T22:44:13Z

I'm actually wondering if this should be renamed kenwood_newlive.py or something... I'm starting to consider doing my TS-B2000 and maybe the TS-940S with this driver. Using the menus on the B2000 is a huge pain and since it's always connected to my computer, it would be nice to be able to set it up using Chirp instead. The 940 I'm not as sure about, I tend to use HF memories a lot differently than I do VHF+ ones.

Likely a change for a follow-on commit though.

W8BSD · 2023-06-25T16:31:48Z

TS-2000 conversion in my TS-B2000-Update branch.

W8BSD · 2023-06-26T05:47:48Z

Ok, I think that's "done" now barring TM-D700 testing, It's now enough to support the wildly different TS-2000, so it seems like the features are nailed down nicely. I'll wait to mark it as ready to review until I actually write a useful commit message.

W8BSD · 2023-06-29T01:34:22Z

That's very interesting, SM is the S-Meter reading, implying that AI mode got enabled somehow... do you know if squelch was open at the time?

W8BSD · 2023-06-29T01:45:54Z

I've pushed a change the specifically ignores "SM " lines, but I suspect that either AI is briefly enabled, so pretty much anything can be received, or perhaps there's a firmware bug resulting in the wrong command return.

kk7ds · 2023-06-29T02:10:36Z

Same deal with 10ceafdc:

DEBUG: PC->RADIO: CH 1
DEBUG: RADIO->PC: 'SM 0,00'
ERROR: Failed to run <RadioJob@0>set_settings([<chirp.settings.RadioSettingGroup object at 0x155d09a50>, <chirp.settings.RadioSettingGroup object at 0x155d09840>, <chirp.settings.RadioSettingGroup object at 0x155d09690>, <chirp.settings.RadioSettingGroup object at 0x14a4f4cd0>, <chirp.settings.RadioSettingGroup object at 0x14a4f4730>],)=None
Traceback (most recent call last):
  File "/Users/dan/Documents/chirp.git/chirp/wxui/radiothread.py", line 72, in dispatch
    self.result = getattr(radio, self.fn)(*self.args, **self.kwargs)
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 1197, in set_settings
    self.set_settings(element)
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 1197, in set_settings
    self.set_settings(element)
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 1201, in set_settings
    self._set_setting(name, sdata, element)
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 1049, in _set_setting
    self._kenwood_set_bool(name, value)
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 875, in _kenwood_set_bool
    return self._kenwood_set(cmd, str(int(value)))
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 872, in _kenwood_set
    raise errors.RadioError("Radio refused to set %s" % cmd)
chirp.errors.RadioError: Radio refused to set CH
DEBUG: Radio finished <RadioJob@0>set_settings([<chirp.settings.RadioSettingGroup object at 0x155d09a50>, <chirp.settings.RadioSettingGroup object at 0x155d09840>, <chirp.settings.RadioSettingGroup object at 0x155d09690>, <chirp.settings.RadioSettingGroup object at 0x14a4f4cd0>, <chirp.settings.RadioSettingGroup object at 0x14a4f4730>],)=RadioError('Radio refused to set CH')
INFO: Editor status: Saved settings

No S-Meter deflection for sure.

W8BSD · 2023-06-29T02:14:08Z

Heh...

if result[0:2] == 'SM ':

Unlikely... fixed.

chirp/drivers/kenwood_d7.py

kk7ds · 2023-06-29T02:15:28Z

chirp/drivers/kenwood_d7.py

+
+# It would be nice to have this as an array so it's grouped in the right
+# Unfortunately, when we do that, we can't have labels for each line
+class KenwoodD7Position(RadioSettingGroup):


These aren't usually subclassed like this. It's not really a problem and they generally don't change much, but none of the other drivers do it (that I know of) so it kinda puts you in being-different-land. The usage is pretty straightforward, so I don't anticipate problems, but it's also one of those things where we won't know until someone tries to use it because it's a live driver. So just FYI...

Oh, this is about the RadioSetting subclasses, not the comment... 😄

Yeah, this seemed like the cleanest way to bundle all the code together and keep the _SETTINGS dict as the only thing the individual drivers needed. I've actually made the settings "stuff" a mixin that I'll be using for a TS-2000 overhaul I've hacked up that I'm very happy with (which is also much faster, and has all the menu settings). I'll try to get as many live radios using it as possible so it's noticed quickly. 😉

kk7ds · 2023-06-29T02:16:44Z

chirp/drivers/kenwood_d7.py

+    _DISABLE_AI_CMD = ('AI', '0')
+    _DUPLEX = {0: "", 1: "+", 2: "-"}
+    _HAS_NAME = True
+    _LOCK = threading.Lock()


Is this necessary? The RadioThread should be handling dispatching one request at a time to the driver...

I only have it there because it was in there before (and is still in kenwood_live). Happy to remove it.

Ah, sorry, it's been a long time since I looked at this stuff. Should be good to remove it.

Turns out it's needed so I can click the settings tab while the memories are loading.

chirp/drivers/kenwood_d7.py

kk7ds · 2023-06-29T02:27:59Z

chirp/drivers/kenwood_d7.py

+        name = entry.get_name()
+        setting = self._get_setting_data(name)
+        # TODO: It would be nice to bump_wait_dialog here...
+        #       Also, this is really only useful for the cli. :(


Yeah, unfortunately, .. hmm, what if you make this a generator? I might be able to make the UI iterate that and do the bumping for you as you read the settings out.

Let me play with this idea (tomorrow) and if it seems good I'll show you the python magic to make this effectively a coroutine that generates pieces of the list as the UI consumes them, thus avoiding hanging up the UI while you pull these all out.

chirp/drivers/kenwood_d7.py

kk7ds · 2023-06-29T02:32:21Z

I'll test this updated one against the radio in a bit..

kk7ds · 2023-06-29T02:44:47Z

Probably just a typo, but from f2a996ae:

|          | 1.0% Fetching Dual                          DEBUG: PC->RADIO: DL
DEBUG: RADIO->PC: 'DL 1'
DEBUG: PC->RADIO: DL 1
DEBUG: RADIO->PC: 'DL 1'
ERROR: Failed to run <RadioJob@10>get_settings(,)=None
Traceback (most recent call last):
  File "/Users/dan/Documents/chirp.git/chirp/wxui/radiothread.py", line 72, in dispatch
    self.result = getattr(radio, self.fn)(*self.args, **self.kwargs)
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 1161, in get_settings
    self._refresh_settings()
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 1029, in _refresh_settings
    self._refresh_setting(entry)
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 1017, in _refresh_setting
    value = self._kenwood_get(name)[1]
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 837, in _kenwood_get
    self._do_prerequisite(cmd, True, True)
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 705, in _do_prerequisite
    self._set_setting(prereq, sdata, entry)
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 1054, in _set_setting
    self._kenwood_set_bool(name, value)
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 880, in _kenwood_set_bool
    return self._kenwood_set(cmd, str(int(value)))
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 875, in _kenwood_set
    if self._kenwood_set_success(cmd, modcmd, value, response):
NameError: name 'response' is not defined
DEBUG: Radio finished <RadioJob@10>get_settings(,)=NameError("name 'response' is not defined")
INFO: Editor status: Retrieved settings
ERROR: Context raised unexpected_exception
Traceback (most recent call last):
  File "/Users/dan/Documents/chirp.git/chirp/wxui/settingsedit.py", line 47, in _initialize
    raise job.result
  File "/Users/dan/Documents/chirp.git/chirp/wxui/radiothread.py", line 72, in dispatch
    self.result = getattr(radio, self.fn)(*self.args, **self.kwargs)
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 1161, in get_settings
    self._refresh_settings()
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 1029, in _refresh_settings
    self._refresh_setting(entry)
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 1017, in _refresh_setting
    value = self._kenwood_get(name)[1]
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 837, in _kenwood_get
    self._do_prerequisite(cmd, True, True)
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 705, in _do_prerequisite
    self._set_setting(prereq, sdata, entry)
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 1054, in _set_setting
    self._kenwood_set_bool(name, value)
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 880, in _kenwood_set_bool
    return self._kenwood_set(cmd, str(int(value)))
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 875, in _kenwood_set
    if self._kenwood_set_success(cmd, modcmd, value, response):
NameError: name 'response' is not defined
Traceback (most recent call last):
  File "/opt/homebrew/lib/python3.10/site-packages/wx/core.py", line 3427, in <lambda>
    lambda event: event.callable(*event.args, **event.kw) )
  File "/Users/dan/Documents/chirp.git/chirp/wxui/settingsedit.py", line 47, in _initialize
    raise job.result
  File "/Users/dan/Documents/chirp.git/chirp/wxui/radiothread.py", line 72, in dispatch
    self.result = getattr(radio, self.fn)(*self.args, **self.kwargs)
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 1161, in get_settings
    self._refresh_settings()
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 1029, in _refresh_settings
    self._refresh_setting(entry)
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 1017, in _refresh_setting
    value = self._kenwood_get(name)[1]
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 837, in _kenwood_get
    self._do_prerequisite(cmd, True, True)
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 705, in _do_prerequisite
    self._set_setting(prereq, sdata, entry)
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 1054, in _set_setting
    self._kenwood_set_bool(name, value)
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 880, in _kenwood_set_bool
    return self._kenwood_set(cmd, str(int(value)))
  File "/Users/dan/Documents/chirp.git/chirp/drivers/kenwood_d7.py", line 875, in _kenwood_set
    if self._kenwood_set_success(cmd, modcmd, value, response):
NameError: name 'response' is not defined

W8BSD · 2023-06-29T04:17:10Z

Probably just a typo, but from f2a996a:

Fixed, thanks.

W8BSD · 2023-06-29T17:11:38Z

I'm really not sure how the style checker wants me to indent those lines it's flagging... it looks properly visually indented to me...

kk7ds · 2023-06-29T22:10:21Z

chirp/drivers/kenwood_d7.py

+        "AIP": _BOOL,
+        "ARL": {'type': 'map',
+                'map': (('Off', '0000'),) +
+                       tuple([('%i' % x, '%04i' % x)


I think it's complaining because it thinks this line belongs to the same level as 'map', but that would look confusing and weird, so don't do that. Convention would be to put the tuple( at the end of the line above and then go in four spaces and I think it'll be happy:

'map': (('Off', '0000'),) + tuple( [('%i' % x, '%04i' % x) for x ...

Yeah, that takes care of that one, but the other two aren't in a tuple(), they're just straight concatenation. I'll try going in four spaces instead of visual alignment and see what it does.

Still complaining, moving back to what looks right to me for now.

Removes dependency on kenwood_live, and implements a separate radio class for D7 series radios. Moves globals into the classes - This allows finer control with subclassing Clean up I/O - Don't require a timeout after each command, speeding up access significantly - Read all "garbage" bytes at the start rather than "just" 25 Add generic support for special channels - Used for scan memories and call memories in the D7s Makes settings easier to implement - Data-driven description of settings with built-in support for bools, ints, strings, lists, and maps, which is enough for most settings - Also supports custom classes for more complex settings, used for positions, DTMF memories, and programmable VFOs currently - Allows for settings to depend on other settings Used for automatic simplex check and tone alert settings Add full menu structure from the manuals. - All settings listed in the manuals that can be controled via serial port are now exposed in Chirp

kk7ds · 2023-06-30T22:05:40Z

Rebased and fixed the style thing. AFAICT, this is better than what we have right now, so I think it makes sense to merge this and iterate from here, especially since the diff is large enough to cause github's web UI heartache. I think the _LOCK thing can be removed now because of how chirp-next works, but I'll let you validate with your radios before we do it.

Anything else you know of outstanding that we need to resolve before we move to this?

Also, were you saying the TS-2000 stuff could be based on the newer of these? I've never had hands on one of those before but I know there are still some issues: https://chirp.danplanet.com/issues/10262

W8BSD · 2023-06-30T23:25:59Z

Rebased and fixed the style thing. AFAICT, this is better than what we have right now, so I think it makes sense to merge this and iterate from here, especially since the diff is large enough to cause github's web UI heartache.

Sounds good to me. I've been amending and force-pushing a single branch through this review because I've kept telling myself I was almost done, so it will be good to get away from that.

I think the _LOCK thing can be removed now because of how chirp-next works, but I'll let you validate with your radios before we do it.

It's still needed so you can load the settings tab while the memories are loading.

Anything else you know of outstanding that we need to resolve before we move to this?

Nope, everything else I'm considering is new features, so there's no need to hold this up for that.

Also, were you saying the TS-2000 stuff could be based on the newer of these? I've never had hands on one of those before but I know there are still some issues: https://chirp.danplanet.com/issues/10262

Yeah, I've actually got a quick hack for it working, but I want to test it a lot more since I was mostly concentrating on the settings. I have a TS-B2000 as my daily driver, so it was a natural thing to play with. I'll have to update it for the latest changes, but I can likely have it in shape for a PR in a week or two.

To do the TS-2000, I'm actually pulling out the settings stuff as a mixin, and overriding a bunch of methods. I'll likely do a PR for the mixin fairly soon, but then add a superclass (kenwood_newlive) that will be shared by this new D7 and the TS-2000 which will help clean up both... aside from being request/response in ASCII, the two protocols are quite different, so basing them on the same class is a bit silly.

kk7ds · 2023-06-30T23:32:02Z

It's still needed so you can load the settings tab while the memories are loading.

Are you saying that because of the comment or because you tested and found it broken? If the latter, I should look into why the UI isn't using the thread for that because it likely means other live drivers are broken.

Yeah, I've actually got a quick hack for it working, but I want to test it a lot more since I was mostly concentrating on the settings. I have a TS-B2000 as my daily driver, so it was a natural thing to play with. I'll have to update it for the latest changes, but I can likely have it in shape for a PR in a week or two.

Sweet, I know some folks will be happy. Please charge anything against that issue. I forgot to amend your commit message for this one, but "Fixes: #10262" will link the commit to the bug in the tracker.

Thanks!

W8BSD · 2023-07-01T02:32:05Z

Are you saying that because of the comment or because you tested and found it broken? If the latter, I should look into why the UI isn't using the thread for that because it likely means other live drivers are broken.

I added the comment after I tested it.

I assume two threads are being used, one for the memories and the other for the settings... when one thread drains the pipe and sends the command, the other thread needs to hold off until after the first one reads the reply.

kk7ds · 2023-07-01T02:42:32Z

I added the comment after I tested it.

Ah, okay I assumed it was there already.

I assume two threads are being used, one for the memories and the other for the settings... when one thread drains the pipe and sends the command, the other thread needs to hold off until after the first one reads the reply.

Yeah, but there's a prioritized queue and thread consumer that the UI uses for memories and should be using to do things like get_settings() as well, that assumes one thing happens against the radio at a time. Anyway, I'll look into it, I was just curious if you had actually tried.

kk7ds · 2023-07-01T04:14:47Z

Ah, we are actually starting multiple threads, but we shouldn't be. Most live radios do not have settings so this has gone unnoticed in the new UI, but that's broken. It also explains another problem I was having with a different radio a few weeks ago which I never got back to (it's a weird radio for other reasons). I'll work on that.

W8BSD mentioned this pull request Jun 23, 2023

Add many more settings to TH-D7G #665

Closed

W8BSD force-pushed the KenwoodD7Settings branch 3 times, most recently from 02fd4bf to 11edeab Compare June 23, 2023 19:03

W8BSD force-pushed the KenwoodD7Settings branch 5 times, most recently from a5d554a to c1188e3 Compare June 24, 2023 04:16

W8BSD force-pushed the KenwoodD7Settings branch 8 times, most recently from a7770fe to 4d6da02 Compare June 24, 2023 22:40

W8BSD force-pushed the KenwoodD7Settings branch from 4d6da02 to b31afed Compare June 25, 2023 16:27

W8BSD force-pushed the KenwoodD7Settings branch 4 times, most recently from a1d0703 to 5ca4670 Compare June 26, 2023 05:43

W8BSD force-pushed the KenwoodD7Settings branch 3 times, most recently from 18be229 to 97a65af Compare June 26, 2023 16:53

W8BSD force-pushed the KenwoodD7Settings branch from 35ac9c5 to 10ceafd Compare June 29, 2023 01:43

W8BSD force-pushed the KenwoodD7Settings branch from 10ceafd to f2a996a Compare June 29, 2023 02:13

kk7ds reviewed Jun 29, 2023

View reviewed changes

W8BSD force-pushed the KenwoodD7Settings branch from f2a996a to 97fc42f Compare June 29, 2023 04:16

W8BSD force-pushed the KenwoodD7Settings branch 4 times, most recently from ffb6e23 to 2278760 Compare June 29, 2023 17:10

kk7ds reviewed Jun 29, 2023

View reviewed changes

W8BSD force-pushed the KenwoodD7Settings branch 3 times, most recently from 1f801df to 58e6d14 Compare June 30, 2023 14:53

kk7ds force-pushed the KenwoodD7Settings branch from 58e6d14 to 350e393 Compare June 30, 2023 22:02

kk7ds force-pushed the KenwoodD7Settings branch from 350e393 to 4388e86 Compare June 30, 2023 22:02

kk7ds merged commit 3506133 into kk7ds:master Jun 30, 2023
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Overhaul of Kenwood D7 Driver #674

Overhaul of Kenwood D7 Driver #674

W8BSD commented Jun 23, 2023 •

edited

Loading

W8BSD commented Jun 23, 2023

W8BSD commented Jun 24, 2023

W8BSD commented Jun 24, 2023

W8BSD commented Jun 25, 2023 •

edited

Loading

W8BSD commented Jun 26, 2023

W8BSD commented Jun 29, 2023

W8BSD commented Jun 29, 2023 •

edited

Loading

kk7ds commented Jun 29, 2023

W8BSD commented Jun 29, 2023

kk7ds Jun 29, 2023

W8BSD Jun 29, 2023

W8BSD Jun 29, 2023

kk7ds Jun 29, 2023

W8BSD Jun 29, 2023

kk7ds Jun 29, 2023

W8BSD Jun 29, 2023

kk7ds Jun 29, 2023

kk7ds Jun 29, 2023

kk7ds commented Jun 29, 2023

kk7ds commented Jun 29, 2023

W8BSD commented Jun 29, 2023

W8BSD commented Jun 29, 2023

kk7ds Jun 29, 2023

W8BSD Jun 30, 2023

W8BSD Jun 30, 2023

kk7ds commented Jun 30, 2023

W8BSD commented Jun 30, 2023

kk7ds commented Jun 30, 2023

W8BSD commented Jul 1, 2023

kk7ds commented Jul 1, 2023

kk7ds commented Jul 1, 2023

Overhaul of Kenwood D7 Driver #674

Overhaul of Kenwood D7 Driver #674

Conversation

W8BSD commented Jun 23, 2023 • edited Loading

W8BSD commented Jun 23, 2023

W8BSD commented Jun 24, 2023

W8BSD commented Jun 24, 2023

W8BSD commented Jun 25, 2023 • edited Loading

W8BSD commented Jun 26, 2023

W8BSD commented Jun 29, 2023

W8BSD commented Jun 29, 2023 • edited Loading

kk7ds commented Jun 29, 2023

W8BSD commented Jun 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kk7ds commented Jun 29, 2023

kk7ds commented Jun 29, 2023

W8BSD commented Jun 29, 2023

W8BSD commented Jun 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kk7ds commented Jun 30, 2023

W8BSD commented Jun 30, 2023

kk7ds commented Jun 30, 2023

W8BSD commented Jul 1, 2023

kk7ds commented Jul 1, 2023

kk7ds commented Jul 1, 2023

W8BSD commented Jun 23, 2023 •

edited

Loading

W8BSD commented Jun 25, 2023 •

edited

Loading

W8BSD commented Jun 29, 2023 •

edited

Loading