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

Squelch usability improvements #186

Merged
merged 4 commits into from Dec 7, 2021
Merged

Squelch usability improvements #186

merged 4 commits into from Dec 7, 2021

Conversation

tmiw
Copy link
Collaborator

@tmiw tmiw commented Nov 12, 2021

This PR contains the following:

  1. Adds an additional decimal place to the current SNR to avoid confusion when signals are near squelch level (resolves https://github.com/drowe67/codec2/issues/238).
  2. In multi-RX mode, per-mode squelch levels are now relative to the mode that can support the weakest signals (currently 700D). For example, setting the squelch to -2.0dB will result in the application telling Codec2 to set 700E's squelch to 1.0dB instead. This is because the minimum SNR for 700D is -2.0dB while the minimum for 700E is 1.0dB per the specs. As a result, blips and unwanted false decodes are dramatically reduced in multi-RX mode.

@Tyrbiter
Copy link

Tyrbiter commented Nov 12, 2021

SNR now starts off reading 0.0 at program open, once the modem is started it goes back to reading in whole dB increments, there is no decimal point or 0.1dB shown. SNR sits at -4dB, it jumps typically to 0dB and with squelch set at -0.5dB in 700E mode there is still quite a lot of short audio false decodes appearing in my headphones.

I have multi-rx multi-thread enabled.

Edit: scratch that, the git checkout of the squelch-ux had an error and I built the wrong thing. Now testing with a correctly built version. Sorry for the noise.

@Tyrbiter
Copy link

I'm uncertain about the squelch level changes, at least about the labelling aspect. I wonder whether it would be better if a 0dB squelch setting translated into the lowest spec'd SNR for the mode. That way the - and + adjustment is clearer about what is being done, as it stands I find it confusing. More thoughts on this would be sensible, it could just be that I am in a minority of 1.

@tmiw
Copy link
Collaborator Author

tmiw commented Nov 14, 2021

I'm uncertain about the squelch level changes, at least about the labelling aspect. I wonder whether it would be better if a 0dB squelch setting translated into the lowest spec'd SNR for the mode. That way the - and + adjustment is clearer about what is being done, as it stands I find it confusing. More thoughts on this would be sensible, it could just be that I am in a minority of 1.

I'm not sure how doing that would impact those with multi-RX off. At least with the current approach, the squelch setting works the same as it always has except when multi-RX is on. @drowe67, any thoughts on this?

@Tyrbiter
Copy link

I think changing to a +/- display would help under all circumstances. I don't think most people want to have to change the squelch setting with mode. I don't know if this is currently remembered by mode, I usually have multi-rx enabled. Previously I spent a lot of time fiddling with the squelch setting, the new calculation of SNR/squelch interaction is great for suppressing the false decodes, it's just the labelling I am having difficulty with.

Perhaps displaying the offset to actual squelch SNR when not using multi-rx? It doesn't make sense when multi-rx is on of course. And call the setting squelch offset? Naturally this would need some explanation in the user manual.

@drowe67
Copy link
Owner

drowe67 commented Nov 14, 2021

I'm uncertain about the squelch level changes, at least about the labelling aspect. I wonder whether it would be better if a 0dB squelch setting translated into the lowest spec'd SNR for the mode. That way the - and + adjustment is clearer about what is being done, as it stands I find it confusing. More thoughts on this would be sensible, it could just be that I am in a minority of 1.

I'm not sure how doing that would impact those with multi-RX off. At least with the current approach, the squelch setting works the same as it always has except when multi-RX is on. @drowe67, any thoughts on this?

Nope, but maybe let a few people try a test release and see how they go.

@tmiw
Copy link
Collaborator Author

tmiw commented Nov 15, 2021

Nope, but maybe let a few people try a test release and see how they go.

Hmm, should #184 and #187 be merged before I do that? Both have fixes for build errors on Windows and macOS.

@tmiw
Copy link
Collaborator Author

tmiw commented Nov 28, 2021

Merged this and #189 into v1.6.2-test to make test build generation easier. Should have something out tonight for initial testing hopefully.

@Tyrbiter
Copy link

Built this and running it on Fedora 35, seems to be essentially identical to my local hacked build. Certainly ready for wider testing.

@tmiw
Copy link
Collaborator Author

tmiw commented Dec 6, 2021

@drowe67, I haven't heard any issues with this change so we can probably review/merge this PR now.

default:
return 0.0f;
}
}
void FreeDVInterface::start(int txMode, int fifoSizeMs, bool singleRxThread, bool usingReliableText)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This table could be a freedv_api.h getter type function, ie a property returned by the FreeDV API. But that would mean a codec2 PR so not really necessary right now 🙂

@drowe67
Copy link
Owner

drowe67 commented Dec 6, 2021

Looks good @tmiw and it sounds like it's working for end users. Pls feel to merge when you are ready.

Is there anything about the operation of this feature that end users should know about, e.g. by a comment in the manual? Or do you feel it's operation is transparent?

@tmiw
Copy link
Collaborator Author

tmiw commented Dec 7, 2021

Is there anything about the operation of this feature that end users should know about, e.g. by a comment in the manual? Or do you feel it's operation is transparent?

Operation should be transparent but I added some info on how it works to the manual.

@tmiw tmiw merged commit 2e799eb into master Dec 7, 2021
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.

Squelch incorrectly (?) triggers when SNR is the same as squelch level
3 participants