Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to add a delay after starting TX and before ending TX. #618

Merged
merged 12 commits into from Dec 24, 2023

Conversation

tmiw
Copy link
Collaborator

@tmiw tmiw commented Dec 10, 2023

Resolves #609 by adding a configuration option to allow TX audio to be delayed after starting TX and on ending TX audio (before returning to RX).

UI for new option:

Screenshot 2023-12-10 at 10 41 42 AM

@barjac
Copy link

barjac commented Dec 15, 2023

Seems perfect here :) Great! Now to test the noise cancelling device when I have made a box for it.
Rather than connecting up a 'scope I used 1000ms to test and it seemed about right :)
The entry box restricts the delay visibility to less than 3 chars, maybe it should be wider?

Screenshot_20231215_234826

Tested at #eb4f0

@tmiw
Copy link
Collaborator Author

tmiw commented Dec 16, 2023

The entry box restricts the delay visibility to less than 3 chars, maybe it should be wider?

How about now?

@barjac
Copy link

barjac commented Dec 16, 2023

The entry box restricts the delay visibility to less than 3 chars, maybe it should be wider?

How about now?

Yes that now allows for 7 digits on my screen - certainly more than adequate!

I'm seeing something a bit odd after doing some more testing.
I am sending the audio to my headphones to test and watching the 'Send' led on the rig which is using rigctld for PTT.
With 1000ms delay the tail delay is obviously working but appears to be less than 1 second. With the delay set to 300ms the start delay appears correct but the tail delay seems to be missing, the tones and the rig 'Send' led are simultaneous as far as I can judge by eye and ear.
Setting the delay to '0' I can see that the rig switches before the audio stops, so it would seem that the audio is stopped after the PTT stop signal has been sent to the radio! I would call this a bug. Surely the audio should be stopped and then the PTT stop command sent if there is likely to be any delay in the program.
I need to set up a 'scope on the rig PTT aux output and the audio input to get some real figures on this.

@barjac
Copy link

barjac commented Dec 16, 2023

Thinking more about this, does the PTT 'stop event' stop input to the encoder and also send the PTT stop signal to hamlib at the same time? If so I assume that the encoding will continue for some time before completing, so that the end of audio out will be delayed by the time taken to encode. Could this explain the delay that I am seeing between the PTT at the rig and the audio output?
If this is the case then maybe the real end of audio out should be the start of the PTT tail delay.
Of course I could be totally wrong :)

@tmiw
Copy link
Collaborator Author

tmiw commented Dec 16, 2023

Thinking more about this, does the PTT 'stop event' stop input to the encoder and also send the PTT stop signal to hamlib at the same time? If so I assume that the encoding will continue for some time before completing, so that the end of audio out will be delayed by the time taken to encode. Could this explain the delay that I am seeing between the PTT at the rig and the audio output? If this is the case then maybe the real end of audio out should be the start of the PTT tail delay. Of course I could be totally wrong :)

I made a minor change that might help. Here's a screenshot of the SDR program for my radio (with delay set to 5000ms to make it easier to spot the delay):

Screenshot 2023-12-16 at 1 55 06 PM

(Basically, I think a specific variable that gets set during stop was getting cleared too early, which might explain the behavior you've seen.)

@barjac
Copy link

barjac commented Dec 17, 2023

@tmiw I don't quite understand what you are attempting to show me in the above images. I can see that there is a ~5sec delay but the longer the delay the harder it is to see the audio delay that we are trying to diagnose.

The latest change in #cd8db does not appear to have improved the situation.

To visibly detect a PTT delay after RF out stops (by watching the rig 'send' led and the rig power out), I need to use a delay setting greater than 250ms, so it is still needing to mask a long delay in the stopping of the audio.

This audio delay must also be over and above any delay in rigctld communication and the rig response to it.

Is it possible to not send the PTT stop command to rigctld (or wherever) until the encoding process has actually finished? Then any delay added by this feature will have the same effect on PTT stop as it does on PTT start.

@tmiw
Copy link
Collaborator Author

tmiw commented Dec 19, 2023

I used GIMP to get some measurements in pixels between PTT on and audio beginning to go out (and likewise, between audio termination and PTT off) from the above screenshot:

screenshot with measurements

These end up corresponding to the following delays:

PTT on: 141/126 * 5 = ~5.6 seconds
PTT off: 113/126 * 5 = ~4.5 seconds

(Note that the intended delay was supposed to be 5 seconds in the previous test.)

So yeah, the wait for PTT off does need to be extended a bit. I'll experiment and see if I can get that any better.

@tmiw
Copy link
Collaborator Author

tmiw commented Dec 19, 2023

I think there was an issue with how we're determining whether audio output completes before beginning the wait at the end of TX. I just checked in a minor tweak to that that seems to get the end wait a lot closer to what's configured:

Screenshot 2023-12-18 at 11 23 07 PM

(~5.15s vs configured 5s)

Hopefully that helps for you?

@barjac
Copy link

barjac commented Dec 20, 2023

This just gets more confusing. :
Tonight I have set up a storage 'scope to monitor the auxiliary TX/RX relay contacts (intended for amplifier switching) and the RF output which I finally did by rectifying a sample of the RF out from the radio. This 'scope does not like RF as it mixes with the sampling frequency and produces odd effects. Even the rectified RF looks rather odd but for this test it does what is needed.

Top trace is PTT (active low) and the bottom indicates RF (ignore the rubbish trace during TX).
All screen shots are at 500ms/div.

I initially tested the latest #627d2 build and discovered that with the delay setting at 0 there was a stop delay of about 600ms already there:

PTT_RF_0_627d2

I then repeated with a delay setting of 200ms which did add around 200ms to the stop delay but only about 50ms at the start.

PTT_RF_200_627d2

I then went back to a version at random (#87588) before you started to add a delay:

PTT_RF_87588

That already seems to have about 100ms start and around 200ms stop delays as it is on my setup.

None of the above seem to agree with the other results I was seeing previously, but then I was not using this latest version.

If I get chance I will test the previous one this way, just to confirm the results I was seeing before.

@barjac
Copy link

barjac commented Dec 20, 2023

I decided to do it now :
Using cd8db I am seeing this with a setting of 0ms:

PTT_RF_0_cd8db

And this with a setting of 200ms:

PTT_RF_200_cd8db

OK must get to bed! I will try to draw some conclusions as to what happened previously with this version tomorrow :\

@tmiw
Copy link
Collaborator Author

tmiw commented Dec 20, 2023

Yeah, let me know how the additional testing goes. At least with my setup, the timings look okay. :(

Maybe it'd be worthwhile to get more people (e.g. @Tyrbiter) trying this out?

@tmiw
Copy link
Collaborator Author

tmiw commented Dec 22, 2023

As an experiment (and to improve UI responsiveness with long delays), I adjusted how the delays were being done (i.e. forcing yields to handle UI events and using std::chrono/std::thread instead of wxWidgets to kick off the delay). Looks like going from TX->RX is right on target while RX->TX has a ~300-400ms delay now:

Screenshot 2023-12-22 at 12 53 31 AM

Though TBH, I think it's better to be longer than expected than shorter, especially when sensitive hardware comes into the picture. Note that some of the RX->TX delay is due to 700D (for example) handling 160ms long frames, so at least some of the delay on that side is unavoidable.

@barjac
Copy link

barjac commented Dec 22, 2023

I tested the ms-reporter-msg branch build I was using for the other PR tests and it gives a sensible result tested this way:

photo_2023-12-22_10-11-45

There is no visible start delay and the 100ms stop delay I think can be attributed to the rig's aux PTT relay dropout time, as it will no doubt have a diode across the coil.

I was thinking that maybe separate variables could be used for start and stop delays. In my case I suspect that a short stop delay and longer start will be needed.

However you now mention that different modes may produce different encoding delays, so I maybe should check whether I can see this effect.

I will first build this latest version and test it with and without delays applied and in different modes, hopefully tonight.

@barjac
Copy link

barjac commented Dec 22, 2023

Testing this branch at #f3031 with the delay setting at 0 there is a tail delay of between 500ms and 700ms depending on the mode in use. The start delay is correct at 0.

Looks like going from TX->RX is right on target while RX->TX has a ~300-400ms delay now

This seems opposite to the result I am seeing :( Unless we are using different terminology?

The start delay is going from RX to TX and is between the PTT low (active low) and start of RF out.
The stop delay is going from TX to RX and is the delay between the end of RF out and the PTT high.

So this is somehow adding a very large (~500ms) stop delay compared to the ms-reporter-msg branch that I tested yesterday at 100ms.

Setting the delay to 200ms does add ~200ms to both start and stop delays.

PTT_RF_200_f3031

The only issue is where the extra ~500ms stop is coming from?

@tmiw
Copy link
Collaborator Author

tmiw commented Dec 24, 2023

Testing this branch at #f3031 with the delay setting at 0 there is a tail delay of between 500ms and 700ms depending on the mode in use. The start delay is correct at 0.

Might be worth testing to see if this is the same in analog mode as well.

This seems opposite to the result I am seeing :( Unless we are using different terminology?

The start delay is going from RX to TX and is between the PTT low (active low) and start of RF out. The stop delay is going from TX to RX and is the delay between the end of RF out and the PTT high.

In the screenshots I've posted so far, TX->RX (stop delay) is represented by the topmost red bar/measurement while RX->TX (start delay) is the red bar on the very bottom. The red bar on the right hand side is for scaling (e.g. 63 pixels per 5 seconds in the very last screenshot).

The only issue is where the extra ~500ms stop is coming from?

Depending on the results in analog mode, it might be something Hamlib related (i.e. it's somehow taking longer than expected to send the needed CAT commands to switch back to RX). Besides hamlib, a patch you can try as well is below; I'm not sure if it'll make things better or worse since SDRs do behave differently:

diff --git a/src/ongui.cpp b/src/ongui.cpp
index c6b7741..c7d0e4f 100644
--- a/src/ongui.cpp
+++ b/src/ongui.cpp
@@ -833,13 +833,14 @@ void MainFrame::togglePTT(void) {
         // Trigger end of TX processing. This causes us to wait for the remaining samples
         // to flow through the system before toggling PTT.  Note that there is a 1000ms 
         // timeout as backup.
+        int sample = g_outfifo1_empty;
         endingTx = true;
 
         before = highResClock.now();
         while(true)
         {
             auto diff = highResClock.now() - before;
-            if (diff >= std::chrono::milliseconds(1000) || codec2_fifo_used(g_rxUserdata->outfifo1) == 0)
+            if (diff >= std::chrono::milliseconds(1000) || (g_outfifo1_empty != sample))
             {
                 break;
             }

(this effectively reverts one of the changes I did in the PR so far to determine when the TX side actually finishes outputting audio)

@barjac
Copy link

barjac commented Dec 24, 2023

Testing this branch at #f3031 with the delay setting at 0 there is a tail delay of between 500ms and 700ms depending on the mode in use. The start delay is correct at 0.

Might be worth testing to see if this is the same in analog mode as well.

It's not happening with the ms-reporter-msg branch in the screenshot next but last, so that probably rules out Hamlib as the cause and points to changes here?
I will try to test in analog mode and also with the patch. (Xmas allowing :)

@barjac
Copy link

barjac commented Dec 24, 2023

Testing the ms-reporter-msg branch (while it is installed) in analogue there is a ~1sec start delay but only the 100ms stop delay as referred to earlier:

analogue

@barjac
Copy link

barjac commented Dec 24, 2023

Testing this (ms-tx-rx-delay) branch in analogue the start delay is ~400ms and the stop is ~0 (with a delay setting of 0)

ms-tx-rx-delay_analogue

@barjac
Copy link

barjac commented Dec 24, 2023

Testing this (ms-tx-rx-delay) branch WITH your revert patch in digital mode:
\o/ Yay!

ms-tx-rx-delay_patched

...and with 200ms delay!

ms-tx-rx-delay_200_patched

Super!
Well done and 10/10 for perseverance!

I think this and the Msg PRs deserve a Christmas 1.9.6 release!

Have a very Merry Christmas , you deserve it! :)

EDIT A quick test of analogue with the patch and a 200ms delay setting has about 600ms start and about 250ms stop, so better than it was without the patch. 73

@tmiw
Copy link
Collaborator Author

tmiw commented Dec 24, 2023

Have a very Merry Christmas , you deserve it! :)

Thanks!

@tmiw tmiw merged commit dfdf0c6 into master Dec 24, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants