drowe67 / freedv-gui

GUI Application for FreeDV – open source digital voice for HF radio
https://freedv.org/
GNU Lesser General Public License v2.1
206 stars 52 forks source link

Add option to disable rig control (i.e. only use PTT) #419

Closed tmiw closed 1 year ago

tmiw commented 1 year ago

By request from the digitalvoice list, there should be something in the options to revert to the pre-1.8.10 behavior for Hamlib (as in, frequency and mode are read only and only PTT is touched).

tmiw commented 1 year ago

Closing as it's been implemented now.

barjac commented 1 year ago

I am still getting morse 'L' from the rig on hitting STOP button in freedv which indicates that it has received a mode setting to LSB. Using git master from one hour ago with the new setting unchecked.

tmiw commented 1 year ago

Oops, try the latest in the ms-hamlib-enable-freq-control branch. PR #427 has been opened with those additional changes.

barjac commented 1 year ago

Yes that fixes the change on hitting STOP.

However I am puzzled by what happens on START. The rig is on the right frequency 5366.5kHz and on USB before hitting START. On hitting START the rig switches to VFO/LSB and then quickly back to MEM/USB where it was. Attached is CLI output. freedv_start_stop.txt Any thoughts?

This looks like the part where it happens:

rig_parse_vfo called 2:rig.c(5195):rig_get_split_vfo returning(0) rig_open(1379): Current split=0, tx_vfo=MEM 2:rig.c(2453):rig_get_mode entered netrigctl_set_vfo called netrigctl_set_vfo: cmd='V VFOA ' netrigctl_transaction: called len=7 network_flush called netrigctl_get_mode called, vfo=VFOA netrigctl_transaction: called len=7 network_flush called rig_parse_mode called netrigctl_set_vfo called netrigctl_set_vfo: cmd='V MEM

Tyrbiter commented 1 year ago

Which hamlib version do you have @barjac ?

Changes in hamlib VFO handling have had effects on various Kenwood radios in recent versions, I can't remember exactly when it started but it doesn't affect the TS-890 in hamlib-4.6~git master now.

tmiw commented 1 year ago

As part of the logic to determine whether the radio has multiple VFOs, FreeDV does call rig_get_vfo on connection to the radio:

        if (rig_get_vfo (m_rig, &vfo) == RIG_OK && (m_rig->state.vfo_list & RIG_VFO_B))
        {
            multipleVfos_ = true;
        }

I wouldn't think that would cause the behavior @barjac is seeing but if anyone knows of an alternate method that won't cause that behavior, we can give it a shot.

tmiw commented 1 year ago

I was thinking--maybe the conditional checks need to be swapped? i.e.

if ((m_rig->state.vfo_list & RIG_VFO_B) && rig_get_vfo (m_rig, &vfo) == RIG_OK)

Though again, that doesn't seem like it'd matter. Shouldn't hurt to try making that change and seeing how it goes?

Tyrbiter commented 1 year ago

I don't see why not, I can try it too but I think it would make sense for @barjac to try it first. I think he is using a TS-450S, that is an older generation of Kenwood than I'm using, it is possible that something in hamlib behaves differently when rig_get_vfo is called.

Tyrbiter commented 1 year ago

I have just tried this change, and I don't see anything different on the TS-890, I've tried Start/Stop, Analog on/off, and exiting freedv and restarting it.

I wondered if the && could be replaced with || and then use negations of the individual tests, I presume that the objective is to avoid calling rig_get_vfo somehow, but I do know that it is the only way hamlib can determine if the rig has multiple VFOs so perhaps it can't be avoided, at least the first call to rig_get_vfo must happen even if freedv remembers the result until the rig close happens.

tmiw commented 1 year ago

I don't think that rig_get_vfo is necessarily required to determine if there are multiple VFOs as that's something inherent in the radio's hardware (and presumably can be hardcoded in the respective hamlib driver). It should succeed if we want to explicitly use VFO IDs instead of RIG_VFO_CURR, though.

Maybe something for @barjac to try is to just remove the initial rig_get_vfo call? It'll still potentially get called ~5s later if his radio has multiple VFOs so that should be a good indication whether it's a hamlib thing.

Tyrbiter commented 1 year ago

I don't think that rig_get_vfo is necessarily required to determine if there are multiple VFOs as that's something inherent in the radio's hardware (and presumably can be hardcoded in the respective hamlib driver).

If memory serves, @mdblack98 said it was, perhaps he can comment. He might also know if the TS-450S and the TS-890 would behave differently.

mdblack98 commented 1 year ago

get_vfo_list explicitly returns all known VFOs. TS450SRig command: \get_vfo_listVFOs: VFOA VFOB MEM  TS-890Rig command: \get_vfo_listVFOs: VFOA VFOB  Mike W9MDB

On Saturday, June 10, 2023 at 02:07:04 PM CDT, Brian ***@***.***> wrote:  

I don't think that rig_get_vfo is necessarily required to determine if there are multiple VFOs as that's something inherent in the radio's hardware (and presumably can be hardcoded in the respective hamlib driver).

If memory serves, @mdblack98 said it was, perhaps he can comment. He might also know if the TS-450S and the TS-890 would behave differently.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

Tyrbiter commented 1 year ago

So that's it then, the TS-450S lists MEM as well as VFOs A and B.

That's why @barjac sees different results from me.

I don't see get_vfo_list in the freedv source, so maybe we need it to identify what is available.

tmiw commented 1 year ago

get_vfo_list explicitly returns all known VFOs. TS450SRig command: \get_vfo_listVFOs: VFOA VFOB MEM  TS-890Rig command: \get_vfo_listVFOs: VFOA VFOB 

IIRC the code used to use this but some Linux distros still had old enough hamlib packages where the function didn't exist. I can put it back if there's a way to check hamlib versions during CMake or something.

mdblack98 commented 1 year ago

In older hamlibs the call will just timeout in rigctld. If the call is valid the return is immediate (no delay). So a 100ms timeout or such should work.

In rigctl you just see a blank line.

Mike W9MDB

On Saturday, June 10, 2023 at 03:40:09 PM CDT, Mooneer Salem @.***> wrote:

   get_vfo_list explicitly returns all known VFOs. TS450SRig command: \get_vfo_listVFOs: VFOA VFOB MEM  TS-890Rig command: \get_vfo_listVFOs: VFOA VFOB 

IIRC the code used to use this but some Linux distros still had old enough hamlib packages where the function didn't exist. I can put it back if there's a way to check hamlib versions during CMake or something.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

tmiw commented 1 year ago

Actually, I seem to remember actual compiler errors, not just a matter of rig_get_vfo_list not responding (https://github.com/drowe67/freedv-gui/pull/385#issuecomment-1537279851). In fact, it looks like Debian is still using hamlib 4.0 in stable whereas IIRC rig_get_vfo_list got added sometime afterward.

Speaking of timeouts, though, it might be a good idea to see if I can get that going. So far it's just using whatever the hamlib default timeout/number of retries is; waiting less time before timing out would probably be better for UX IMO.

EDIT: I think I figured it out; something like:

rig_set_conf(m_rig, rig_token_lookup(m_rig, "timeout"), "100"); // in milliseconds
rig_set_conf(m_rig, rig_token_lookup(m_rig, "retry"), "1"); // number of retries

Do these values sound sufficient for hamlib operations or do you guys have other suggestions? I'm thinking we may run into problems on slower machines potentially unless the timeout is higher, but I'm not fully sure how much higher it needs to be.

mdblack98 commented 1 year ago

The return time is virtually immediate -- here's an example call a bad command with verbose output

Rig command: \get_vfo_list2 2023-06-10T22:06:39.182051-0600: rigctl_parse: input_line: \get_vfo_list2 Command '\get_vfo_list2' not found! 2023-06-10T22:06:39.182067-0600: rigctl_parse: called, interactive=1

Less than 1ms.

Mike W9MDB

On Saturday, June 10, 2023 at 09:49:01 PM CDT, Mooneer Salem @.***> wrote:

Actually, I seem to remember actual compiler errors, not just a matter of rig_get_vfo_list not responding (#385 (comment)). In fact, it looks like Debian is still using hamlib 4.0 in stable whereas IIRC rig_get_vfo_list got added sometime afterward.

Speaking of timeouts, though, it might be a good idea to see if I can get that going. So far it's just using whatever the hamlib default timeout/number of retries is; waiting less time before timing out would probably be better for UX IMO.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

tmiw commented 1 year ago

Oh I probably should have clarified. The timeout/retry setting would be for stuff like PTT and frequency/mode changes/queries, not really for grabbing which VFOs exist.

Anyway, I might be able to check for rig_get_vfo_list via CMake. Will investigate more.

tmiw commented 1 year ago

@Tyrbiter, @barjac, try https://github.com/drowe67/freedv-gui/pull/429?

barjac commented 1 year ago

Been AFK for a couple of days. Sorry for delay. I am using hamlib-4.5.5. If I build from latest ms-hamlib-enable-freq-control branch I guess I will get this change?

Tyrbiter commented 1 year ago

You need to merge in ms-hamlib-rig-list as well to include the needed hamlib call.

barjac commented 1 year ago

Testing with master patched with PRs 429 & 430 applied:

Output from $ freedv > fdvdebug2.txt 2>&1 fdvdebug2.txt

The 'Start' button is still causing the radio to switch to VFOA and back to MEM where is was originally (already on correct frequ).

I do see the 'rig.c(7224):rig_get_vfo_list entered' in the attached log. Log covers one Start, one Stop and close.

tmiw commented 1 year ago

If I'm understanding the logs correctly, rig_get_vfo_list seems to be happening after the VFO switch occurs, so it's something in hamlib doing it. @mdblack98 mentioned something in the PR about an option that could be set, so I'm getting more info now.

Tyrbiter commented 1 year ago

In the test with RIG_VFO_B would it help to have an || RIG_MEM assuming that exists?

mdblack98 commented 1 year ago

Some rigs need VFO_MEM for 60M operations for example. It's safest to assume not to do frequency changes when in MEM mode. I have some exception code in Hamlib but for freeDV I'd think just leave things alone if in MEM mode and show MEMORY for frequency.

mdblack98 commented 1 year ago

rig_vfo_list can be at any time.

You say "something in hamlib doing it" -- doing what?

Mike

On Sunday, June 11, 2023 at 12:43:51 PM CDT, Mooneer Salem @.***> wrote:

If I'm understanding the logs correctly, rig_get_vfo_list seems to be happening after the VFO switch occurs, so it's something in hamlib doing it. @mdblack98 mentioned something in the PR about an option that could be set, so I'm getting more info now.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

tmiw commented 1 year ago

Some rigs need VFO_MEM for 60M operations for example. It's safest to assume not to do frequency changes when in MEM mode. I have some exception code in Hamlib but for freeDV I'd think just leave things alone if in MEM mode and show MEMORY for frequency.

Yeah, there are radios that indeed have MEM but also only VFOA, so it wouldn't be quite correct to add MEM to the multipleVfo check.

rig_vfo_list can be at any time. You say "something in hamlib doing it" -- doing what?

Switching the radio to VFOA and back to MEM mode on open. Though it does look like we immediately request current mode and frequency after open, too:

        if (rig_get_vfo (m_rig, &vfo) == RIG_OK && (m_rig->state.vfo_list & RIG_VFO_B))
        {
            multipleVfos_ = true;
        }

        // Get current frequency and mode when we first connect so we can 
        // revert on close.        
        update_from_hamlib_();

(but I wouldn't expect that to force switching between VFO/MEM, especially since that only seems to happen once per @barjac's report.)

BTW @barjac, remind us the radio you're using again?

Tyrbiter commented 1 year ago

It's a TS-450S which is set to 5.3665MHz stored in a memory channel. On starting freedv's modem, even with the CAT control (other than PTT) turned off, the radio switches to VFO A and then back to Memory again. It seems to be rig_get_vfo that does this.

The object of the no-CAT setting is to allow the user to control the radio without any CAT commands other than PTT.

You said yesterday that a TS-450S sends VFO A, VFO B and MEM with rig_get_vfo whereas a TS-890 which sends VFO A, VFO B does not switch to either VFO when the freedv modem starts when the radio is in Memory mode. I think this last is somehow significant because one radio lists memory mode as a 'VFO' whereas the other doesn't.

mdblack98 commented 1 year ago

You also need 

m_rig->state.vfo_list & RIG_VFO_SUB

if (rig_get_vfo (m_rig, &vfo) == RIG_OK && (m_rig->state.vfo_list & RIG_VFO_B || m_rig->state.vfo & RIG_VFO_SUB))

On Sunday, June 11, 2023 at 04:13:09 PM CDT, Mooneer Salem @.***> wrote:

   Some rigs need VFO_MEM for 60M operations for example. It's safest to assume not to do frequency changes when in MEM mode. I have some exception code in Hamlib but for freeDV I'd think just leave things alone if in MEM mode and show MEMORY for frequency.

Yeah, there are radios that indeed have MEM but also only VFOA, so it wouldn't be quite correct to add MEM to the multipleVfo check.

   rig_vfo_list can be at any time. You say "something in hamlib doing it" -- doing what?

Switching the radio to VFOA and back to MEM mode on open. Though it does look like we immediately request current mode and frequency after open, too:

        if (rig_get_vfo (m_rig, &vfo) == RIG_OK && (m_rig->state.vfo_list & RIG_VFOB))         {             multipleVfos = true;         }

        // Get current frequency and mode when we first connect so we can         // revert on close.                 update_fromhamlib();

(but I wouldn't expect that to force switching between VFO/MEM, especially since that only seems to happen once per @barjac's report.)

BTW @barjac, remind us the radio you're using again?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

tmiw commented 1 year ago

Thanks @mdblack98! I updated the PR with that extra logic.

All, pull the latest from ms-hamlib-rig-list and try again?

barjac commented 1 year ago

@tmiw In a patch from current PR #429 I see: + if (mode == m_curMode) Is that a typo? I saw you corrected another 'm_curMode' to 'm_currMode'.

tmiw commented 1 year ago

Yep, that's a typo. The now latest should compile now :)

Tyrbiter commented 1 year ago

Yep, that's a typo. The now latest should compile now :)

I can confirm that it does.

barjac commented 1 year ago

I will test shortly - thanks :)

barjac commented 1 year ago

Building from git master at #aa280d1 patched with current PR #429 I am seeing no operational change.

fdvdebug3.txt

netrigctl_set_vfo: cmd='V VFOA (at line 142 of log) then later: netrigctl_set_vfo: cmd='V MEM (at line 167 of log)

Edit: I have double checked that the patch is correct. (It includes the typo fix). All builds are rpm packages built in a clean chroot and updated with the system package manager, so there is only one freedv version on the system.

I also notice that when the current non-memory vfo is VFO-B The switch on hitting the 'Start' button in FreeDV with the rig set to a memory is still to VFO-A. fdvdebug4.txt

Tyrbiter commented 1 year ago

I see this at line 133 of the first debug file (135 in the second):

2:rig.c(5130):rig_get_split_vfo entered

then starting at line 137 (139 in the second debug file)

rig_parse_vfo called 2:rig.c(5195):rig_get_split_vfo returning(0) rig_open(1379): Current split=0, tx_vfo=MEM

this appears to mean that hamlib thinks the radio is in split mode.

That doesn't seem right to me.

Perhaps @mdblack98 could have a look at this, there have been strange things with hamlib and split mode before.

mdblack98 commented 1 year ago

No -- split=0 means split is not on.

What's happening is when the rig is opened it gets all sort of information from the rig at startup.

Ergo you see a VFO swap occur.

It may be possible for me to make some change that might ignore some things if MEM mode is active. I'm a little reluctant  to do that for fear of breaking other things.

But is the one VFO swap that intolerable?

Mike W9MDB

On Monday, June 12, 2023 at 11:26:54 AM CDT, Brian @.***> wrote:

I see this at line 135 of the first debug file:

2:rig.c(5130):rig_get_split_vfo entered

then starting at line 139

rig_parse_vfo called 2:rig.c(5195):rig_get_split_vfo returning(0) rig_open(1379): Current split=0, tx_vfo=MEM

this appears to mean that hamlib thinks the radio is in split mode.

That doesn't seem right to me.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

Tyrbiter commented 1 year ago

No -- split=0 means split is not on. What's happening is when the rig is opened it gets all sort of information from the rig at startup. Ergo you see a VFO swap occur. It may be possible for me to make some change that might ignore some things if MEM mode is active. I'm a little reluctant to do that for fear of breaking other things. But is the one VFO swap that intolerable?

I don't see this problem with the TS-890 but the TS-450S exhibits it for @barjac

I suppose we would all like to see consistent behaviour but I entirely understand that achieving this is difficult. I looked for a listing of CAT commands for the TS-450S but I couldn't find one. That doesn't help.

barjac commented 1 year ago

Hi Mike, I have a more verbose rigctld output from it's start and continuing after freedv is launched and the modem started at line 412 in the log.

rigctld_freedv_20230612_1816.log

I normally use klog which does work fine alongside freedv when it has been started with the rig in MEM mode. However if I need to QSY to an odd frequency that is not in a memory then on switching to VFO mode a fight starts between klog, freedv and hamlib switching rapidly between MEM and VFO ad infinitum until klog is stopped. I have been thinking that this may be related to what has been discussed here, but so far have only reported it as a klog issue, not wishing to take this off topic and confuse things.

tmiw commented 1 year ago

On a somewhat related note, https://github.com/drowe67/freedv-gui/pull/429/commits/8cb437b01bd3b3182abc322f7719a1abd00ed77b seems to have really messed things up for @Kimberly-McBlaze (https://github.com/drowe67/freedv-gui/issues/431#issuecomment-1587169659) so I'll probably end up reverting that change (especially since it didn't help avoid the VFO switch on startup for @barjac).

Really, though, problems on the hamlib side shouldn't be taking out FreeDV entirely. I have some ideas on improving that but I'll probably have to do some significant refactoring to do so. I'll work on it some more tonight.

barjac commented 1 year ago

That's a shame as further testing just now indicates that the issue with klog seems fixed with PR #429. I can live with the Start mode switch if the klog issue is indeed cured. For the test I was running rigctld as user where it is run as root by systemd normally. I will do much more testing later to confirm.

Tyrbiter commented 1 year ago

On a somewhat related note, 8cb437b seems to have really messed things up for @Kimberly-McBlaze (#431 (comment)) so I'll probably end up reverting that change (especially since it didn't help avoid the VFO switch on startup for @barjac).

I'm fairly neutral on this, I have not seen any problems with the last few versions. Let's see what @tmiw cooks up and try again.

barjac commented 1 year ago

I have discovered something. When testing to create logs I was running:

rigctld -v -vvvvv -m 2003 -r /dev/ttyUSB0 -t 4532 --set-conf=serial_handshake=None

systemd runs: rigctld --vfo -vvvvv -m 2003 -r /dev/ttyUSB0 -T 127.0.0.1 -t 4532 --set-conf=serial_handshake=None ...picking up the options from a config file

I have always understood that -v (single) was the short option for --vfo since I wrote the systemd unit file and config file many years ago, however that now seems wrong and my testing for logs was without the --vfo option which was being applied by systemd.

My testing just now seems to indicate that without --vfo the fight between klog and freedv seems cured and both are playing nice together.

I have never fully understood the --vfo option other than that I was told by Mike that it was needed during some old hamlib bug investigations some years ago.

I still get the momentary VFO switch when hitting Start in freedv when in MEM mode but that is a non issue.

@Mike. Does any of this make sense?

I will re-test a build without the PR #429 patch in light of the above.

Tyrbiter commented 1 year ago

I have always understood that -v (single) was the short option for --vfo since I wrote the systemd unit file and config file many years ago, however that now seems wrong and my testing for logs was without the --vfo option which was being applied by systemd.

It seems the short option for --vfo is -o

Hamlib is in a certain amount of flux, it's clearly difficult to ensure total equivalence with all rig controls in every case.

I don't know whether @tmiw will find a way round this in the freedv code, but it looks like it is possible to get back to the situation before the various problems seen recently without doing too much.

mdblack98 commented 1 year ago

The difference between the TS890 and TS450 is that the TS890 has a targetable mode whereas the TS450 does not.

That means one can get/set mode on the TS890 without VFO swapping  but the TS450 has to swap VFOs to get VFOB mode.

So if you enable --vfo (-o as shown in "rigctl -h")  and ask the TS450 for VFO_B info you will see a VFO swap occur every time.

It's just older rigs don't have the more advanced commands and of course our needs have grown more complex over the years.

Mike W9MDB

On Monday, June 12, 2023 at 04:17:18 PM CDT, Brian @.***> wrote:

   I have always understood that -v (single) was the short option for --vfo since I wrote the systemd unit file and config file many years ago, however that now seems wrong and my testing for logs was without the --vfo option which was being applied by systemd.

It seems the short option for --vfo is -o

Hamlib is in a certain amount of flux, it's clearly difficult to ensure total equivalence with all rig controls in every case.

I don't know whether @tmiw will find a way round this in the freedv code, but it looks like it is possible to get back to the situation before the various problems seen recently without doing too much.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

Tyrbiter commented 1 year ago

A good explanation Mike, I hadn't quite worked out what the targetable feature did, but I do now.

barjac commented 1 year ago

Without PR #429 and --vfo option in the rigctld options hamlib, klog and freedv are now playing fine together whichever vfo is selected on the radio at startup of freedv or klog. I can now switch randomly between memory and VFO mode in either direction. wsjtx also still works when controlling the frequency and with klog logging, so feel free to drop PR #429 :)

tmiw commented 1 year ago

Works for me. I'll close the PR and this bug. The hamlib logic should still be refactored but that can wait.