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

Implement automatic stats reset after sync loss #262

Merged
merged 11 commits into from Sep 24, 2022
Merged

Implement automatic stats reset after sync loss #262

merged 11 commits into from Sep 24, 2022

Conversation

tmiw
Copy link
Collaborator

@tmiw tmiw commented Jul 26, 2022

Feature request to automatically reset stats after sync loss. See https://github.com/drowe67/freedv-gui/discussions/244 for more info.

TODO:

  • Prototype automatic sync reset and ensure that it works properly with hardcoded timeouts.
  • Add configuration UI to enable/disable feature.

@Tyrbiter
Copy link

Tyrbiter commented Aug 3, 2022

I also noticed this https://github.com/drowe67/codec2/issues/182 which makes me wonder if this effort would better wait until a conclusion is reached within codec2.

@tmiw
Copy link
Collaborator Author

tmiw commented Aug 4, 2022

I also noticed this drowe67/codec2#182 which makes me wonder if this effort would better wait until a conclusion is reached within codec2.

Based on previous discussion it seems like we could proceed now if we wanted, unless there's something I'm missing?

@Tyrbiter
Copy link

Tyrbiter commented Aug 4, 2022

I also noticed this drowe67/codec2#182 which makes me wonder if this effort would better wait until a conclusion is reached within codec2.

Based on previous discussion it seems like we could proceed now if we wanted, unless there's something I'm missing?

Ah, OK, yes I looked at the previous PR but didn't realise that there were a couple of changes pulled in as a result of it. I withdraw my comment given this has already been done. I notice that PR182 is still open @drowe67 is that intentional?

@drowe67
Copy link
Owner

drowe67 commented Aug 5, 2022

I also noticed this drowe67/codec2#182 which makes me wonder if this effort would better wait until a conclusion is reached within codec2.

Based on previous discussion it seems like we could proceed now if we wanted, unless there's something I'm missing?

Ah, OK, yes I looked at the previous PR but didn't realise that there were a couple of changes pulled in as a result of it. I withdraw my comment given this has already been done. I notice that PR182 is still open @drowe67 is that intentional?

Yes.

@Tyrbiter
Copy link

Tyrbiter commented Aug 6, 2022

Right. It will be interesting to see what other changes come along.

@tmiw
Copy link
Collaborator Author

tmiw commented Sep 14, 2022

@Tyrbiter, how's this PR been working for you? Is there value in adding config options for the automatic stats reset at all, or is the 10 second timeout sufficient?

@Tyrbiter
Copy link

Tyrbiter commented Sep 14, 2022

Hi @tmiw

So far the 10 second timeout seems to be fairly good, but there are still a few cases where it is a little long. If 2 stations are in a QSO and pass it back quickly then the BER can be confused by this. It's also because on some channels the false sync rate is higher than on others.

How much trouble would it be to give a choice of 2, 5 and 10 seconds?

@tmiw
Copy link
Collaborator Author

tmiw commented Sep 15, 2022

Hi @tmiw

So far the 10 second timeout seems to be fairly good, but there are still a few cases where it is a little long. If 2 stations are in a QSO and pass it back quickly then the BER can be confused by this. It's also because on some channels the false sync rate is higher than on others.

How much trouble would it be to give a choice of 2, 5 and 10 seconds?

Shouldn't be too much trouble. Would this setting need to be one that can be changed while the session's running, or would it be okay if the user had to stop first (like with most other settings in the application)?

@Tyrbiter
Copy link

I think it would be fine to make it only change when the modem is stopped, and possibly have a default of 5 seconds.

@tmiw
Copy link
Collaborator Author

tmiw commented Sep 16, 2022

I think it would be fine to make it only change when the modem is stopped, and possibly have a default of 5 seconds.

Done. It still defaults to 10 seconds but you can go to Tools->Options->Modem to bump it up or down.

@Tyrbiter
Copy link

Have now built and installed this, seems to work but I will need to try it with on air signals to assess it.

One small thing is that with the time set to 10 seconds the modem tab box for the number is a bit too narrow, so if this is the right line of code:

sizerModem->Add(sbSizer_modemstats,0, wxALL|wxEXPAND, 3);

then I assume making it slightly wider needs a tweak to one of the parameters.

@tmiw
Copy link
Collaborator Author

tmiw commented Sep 16, 2022

Have now built and installed this, seems to work but I will need to try it with on air signals to assess it.

One small thing is that with the time set to 10 seconds the modem tab box for the number is a bit too narrow, so if this is the right line of code:

sizerModem->Add(sbSizer_modemstats,0, wxALL|wxEXPAND, 3);

then I assume making it slightly wider needs a tweak to one of the parameters.

Yep, I just widened it a bit.

@Tyrbiter
Copy link

Yep, I just widened it a bit.

Thanks, also notice something similar on the debug tab with the box for the Fifo Size.

@tmiw
Copy link
Collaborator Author

tmiw commented Sep 17, 2022

Yep, I just widened it a bit.

Thanks, also notice something similar on the debug tab with the box for the Fifo Size.

Just made that wider too. I'll merge the PR in a few days if there are no other issues.

@Tyrbiter
Copy link

That has fixed it, just looked through the tabs, the same change is needed for the UDP port number.

@Tyrbiter
Copy link

I'm currently listening to the UK FreeDV net on 60m and I can confirm that using a 5 second stats reset time is much better than 10 seconds, very few stations manage to resume carrier in such a short period so the stats are behaving exactly as they should.

@tmiw
Copy link
Collaborator Author

tmiw commented Sep 17, 2022

That has fixed it, just looked through the tabs, the same change is needed for the UDP port number.

Done. 👍

I'm currently listening to the UK FreeDV net on 60m and I can confirm that using a 5 second stats reset time is much better than 10 seconds, very few stations manage to resume carrier in such a short period so the stats are behaving exactly as they should.

Excellent!

@tmiw tmiw merged commit 848c65c into master Sep 24, 2022
tmiw added a commit that referenced this pull request Oct 8, 2022
tmiw added a commit that referenced this pull request Oct 8, 2022
Add missed changelog entry for PR #262.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants