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 additional error checking when configuring FreeDV Reporter. #535

Merged
merged 2 commits into from
Sep 14, 2023

Conversation

tmiw
Copy link
Collaborator

@tmiw tmiw commented Sep 12, 2023

During testing, I found some instances where FreeDV Reporter was connecting even though the general reporting enable checkbox wasn't checked. This PR fixes those and adds some additional error checking for callsign and locator/grid square.

@Tyrbiter
Copy link

Testing this change shows that it behaves as intended, FreeDV Reporter only shows reporter data when enabled.

@barjac
Copy link

barjac commented Sep 13, 2023

Are you saying that this 'feature' is being removed?

At present with the modem stopped and Tools->Options->Reporting tab->'Enable Reporting' unchecked I can use the Internal reporter to monitor FreeDV activity.

Is this what is being referred to here?
If so why is this undesirable? I find it useful.

Also useful for users who use FreeDV to monitor a remote sdr and want to use the same instance to run the internal reporter without reporting from that machine.

This was part of a recent update and intentional as far as I understood.

Am I misunderstanding this?

@tmiw
Copy link
Collaborator Author

tmiw commented Sep 13, 2023

Are you saying that this 'feature' is being removed?

At present with the modem stopped and Tools->Options->Reporting tab->'Enable Reporting' unchecked I can use the Internal reporter to monitor FreeDV activity.

Is this what is being referred to here? If so why is this undesirable? I find it useful.

Also useful for users who use FreeDV to monitor a remote sdr and want to use the same instance to run the internal reporter without reporting from that machine.

This was part of a recent update and intentional as far as I understood.

Am I misunderstanding this?

See #534 (comment), but in short, a valid FreeDV Reporter configuration (which includes both "Enable FreeDV Reporter" and "Enable Reporting" being checked) is required to be able to view reporting data. The recent change that was merged was intended to allow viewing that data without a running session as long as there's a valid configuration, but it was apparently allowing it for at least some invalid configs. Hope that makes sense?

@Tyrbiter
Copy link

@barjac It appears that the previous state where one could see the reporter data without a valid configuration being set (i.e. with the Enable FreeDV Reporter unchecked) was actually a bug and that has now been corrected.

To allow the modem to run for monitoring on-air signals but not to appear on the reporter while still being able to see all the other stations on it you will have to have a browser window open with qso.freedv.org displayed.

@tmiw
Copy link
Collaborator Author

tmiw commented Sep 14, 2023

@barjac It appears that the previous state where one could see the reporter data without a valid configuration being set (i.e. with the Enable FreeDV Reporter unchecked) was actually a bug and that has now been corrected.

To allow the modem to run for monitoring on-air signals but not to appear on the reporter while still being able to see all the other stations on it you will have to have a browser window open with qso.freedv.org displayed.

Or more accurately, you'll need to use the web version if you don't configure your callsign/locator and check both checkboxes in Tools->Options->Reporting. If FreeDV Reporter is properly configured, though, you'll only appear on there once you push Start (but will be able to view other traffic without starting a session).

@tmiw tmiw merged commit 61911eb into master Sep 14, 2023
2 checks passed
@barjac
Copy link

barjac commented Sep 14, 2023

This has broken the ability to run a freedv instance without reporting but while receiving that also displays the internal reporter.

.....was actually a bug and that has now been corrected.

What damage was it doing?

For me it was providing a desired feature.

Or more accurately, you'll need to use the web version if you don't configure your callsign/locator >and check both checkboxes in Tools->Options->Reporting. If FreeDV Reporter is properly configured, >though, you'll only appear on there once you push Start (but will be able to view other traffic >without starting a session).

We want to use a second machine to monitor a web sdr, so the modem has to be running.
We don't want that machine to report as it clutters the reporter and is invalid as it's a web sdr.
We do want to monitor the Internal Reporter on that machine as it's more compact.

@Tyrbiter
Copy link

Tyrbiter commented Sep 14, 2023

The bug was against @tmiw 's definition of a valid instance with all the fields in the Options->Reporter tab having valid contents. I didn't mean a bug in terms of causing an error or other problem.

A thought - if using a web SDR with the modem running in a second FreeDV instance, will this allow use of the reporter if the transmit devices are set to none? I think it should - it should show up as Receive only. Maybe another optional setting could allow the suppression of Receive only stations in the reporter or maybe only one's own Receive only entry.

I mentioned this previously as being part of the reason for the #417 enhancement to allow easy selection of different configurations on the fly.

@barjac
Copy link

barjac commented Sep 15, 2023

The bug was against @tmiw 's definition of a valid instance with all the fields in the Options->Reporter tab having valid contents. I didn't mean a bug in terms of causing an error or other problem.

What is considered valid?
I can understand that the correct info is needed in order to send reports and that it should not allow reporting without certain fields being filled out.

What I don't understand is why this should affect running the reporter to monitor as when watching the web version.
Before this change I could uncheck 'Enable reporting' and keep 'Enable FreeDV Reporter' checked and there was no problem.

What is the reason behind blocking this usage? Is it purely technical or security or bandwidth or ...?

A thought - if using a web SDR with the modem running in a second FreeDV instance, will this allow use of the reporter if the transmit devices are set to none? I think it should - it should show up as Receive only.

Testing on one machine just now with current master (db05c..) and setting the TX audio to both 'none' still shows 'Receiving' not 'Receive Only' so that has changed at some point.
Showing anything for SDR monitoring only stations is the issue.
In a net with say six stations who work this way, having duplicate entries in the reporters is confusing, and the offenders are always asked to stop reporting the second instance.
If they do that now they will lose the internal reporter on that instance.

Maybe another optional setting could allow the suppression of Receive only stations in the
reporter or maybe only one's own Receive only entry.

Academic now as it's not reporting 'Receive only', but why waste bandwidth sending data that is only going to end up unused.

I mentioned this previously as being part of the reason for the #417 enhancement to allow easy
selection of different configurations on the fly.

That would be useful as machines could be switched between monitor only and RX/TX much easier on the fly, but it does not impact this issue.

@Tyrbiter
Copy link

This commit shows the bulk of the requirements, but there was an earlier one that broke it for me:

27f16f2

I don't think this is set in stone, @tmiw simply wanted to get something with reporter settings validation into this recent release.

I'm sure some more tweaking will happen, it just needs a proper spec for how the logic will work. I think the requested configuration enhancement would form the base for this.

@tmiw
Copy link
Collaborator Author

tmiw commented Sep 16, 2023

Apologies for not responding sooner. There are a few complications to just having FreeDV Reporter working full time regardless of configuration. First off, the current setup UI implies that FreeDV Reporter only works with proper configuration. For instance, note the grayed-out items when "Enable Reporting" isn't checked:

Screenshot 2023-09-15 at 5 15 38 PM

(This is what drove the recent changes to suppress connecting unless both checkboxes are checked and there's a valid callsign/locator.)

Additionally, it is possible for people to install their own instances of FreeDV Reporter (https://bitbucket.org/tmiw/freedv-reporter/src), so that's why there's a separate "hostname" field. I don't think anyone is currently doing this in practice, so this typically gets left as the default. However, it is theoretically possible for someone to clear this field, which would make it impossible to connect anywhere. :)

Anyway, it should be possible to just have a "view only" connection. After all, that's how the web version of FreeDV Reporter works. Just need to do it in a way that doesn't confuse people. Maybe something like this?

  1. Have a separate "FreeDV Reporter" section in the Reporting tab that lets people configure the hostname regardless of whether reporting via FreeDV Reporter is turned on.
  2. Change the labels of the checkboxes to something like "Report via FreeDV Reporter" to indicate that this is for reporting spots (and not simply being connected).
  3. When not running a session, the application is connected to the FreeDV Reporter server in the "view" role (what the web version does). This is regardless of the user's reporting configuration and will gracefully degrade depending on what's not valid (for example, maybe distances get replaced with "N/A" if there's no locator/grid square provided).
  4. When the user pushes Start (and there's a valid reporting configuration), the view-only connection goes away and is replaced with one that actually does report. Otherwise, the same connection from (3) sticks around.

BTW I suspect FreeDV Reporter needs to be cycled in order to properly reflect "Receive Only" status with the latest changes (or a restart of the application should work, too). Before, it was able to get audio device changes before connecting, which is what triggers "Receiving" vs. "Receive Only".

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.

3 participants