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

Grappas/bugfixes #27

Merged
merged 4 commits into from
Jun 10, 2023
Merged

Grappas/bugfixes #27

merged 4 commits into from
Jun 10, 2023

Conversation

grappas
Copy link
Collaborator

@grappas grappas commented Jun 4, 2023

Found few bugs. Plz see commit comments.

Less space taken
it was omitting every odd index of Frequency[i]
if none arguments were given gqrx-scanner performed sweep scan anyway
    malloc() created single struct, sweep was performed and when
    Frequencies[74] was touched it segfaulted into oblivion
@neural75
Copy link
Owner

neural75 commented Jun 5, 2023

Hi @grappas : can you please explain the inner "for loop" for i in Frequencies[] on the ScanFrequenciesInRange()?
The Frequencies array was used for loading the bookmarks only by the ScanBookmarkedFrequenciesInRange(): FREQ_MAX was just the maximum number of bookmarks, if you repurposed it to store the noise_floor squelch level, consider that this array could become very big (depending on the steps). Also, are the other fields initialized or it was just to process the squelch threshold?

Copy link
Owner

@neural75 neural75 left a comment

Choose a reason for hiding this comment

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

Comments on Frequencies array

@@ -1580,6 +1571,16 @@ int main(int argc, char **argv) {
print_usage(argv[0]);
}

if (opt_scan_mode == sweep)
{
size_t freqeuencies_count = (size_t)(((opt_max_freq-opt_min_freq)/opt_scan_bw)+1);
Copy link
Owner

Choose a reason for hiding this comment

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

This could grow very very big in case of large scan. Do you really need to store the noise floor for each step? I was even tempted to remove the GetSquelchLevel from the inner loop (to speed up a bit) and get a value before each round (the user will not change the setting during the scan, usually, so it is not optimized). Can we do something like that to avoid the fine-grained array?

Copy link
Collaborator Author

@grappas grappas Jun 5, 2023

Choose a reason for hiding this comment

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

This could grow very very big in case of large scan. Do you really need to store the noise floor for each step? I was even tempted to remove the GetSquelchLevel from the inner loop (to speed up a bit) and get a value before each round (the user will not change the setting during the scan, usually, so it is not optimized). Can we do something like that to avoid the fine-grained array?

We need to know, for each frequency what noise floor we are dealing with and yeah - each one. If we have ie. UKF NFM 12500 Hz with bandwidth 8500 Hz or USB which can be even narrower and we would mesh it more loosely we would end up with omitting chatter.
In the other hand: lets say 2MHz scanning every 12500 Hz we are ending up with 160 elements, 1856 bytes each (if I remember correctly) we are ending up with 290 KiB of memory space. C'mon ( ͡° ͜ʖ ͡°) it's not that much.
If we go crazy and we wanted to cover whole 800MHz we would end up with 113 MiB.

If we could successfully cooperate with gqrx team and they would manage to implement array of frequencies only then we can discuss about optimization.

It has to be what it is for now.

@grappas
Copy link
Collaborator Author

grappas commented Jun 5, 2023

Hi @grappas : can you please explain the inner "for loop" for i in Frequencies[] on the ScanFrequenciesInRange()? The Frequencies array was used for loading the bookmarks only by the ScanBookmarkedFrequenciesInRange(): FREQ_MAX was just the maximum number of bookmarks, if you repurposed it to store the noise_floor squelch level, consider that this array could become very big (depending on the steps). Also, are the other fields initialized or it was just to process the squelch threshold?

It has to compare signal level to squelch each time and calculate noise_floor for each frequency. We could obviously put some of the functions outside for loop but due to lag (obviously because of querying for signal level each time we are changing frequency) it would be very annoying ie. adjusting squelch in gqrx in-flight.

That and other stuff would be easier if we could access whole visible spectrum at once ( ͡° ͜ʖ ͡°)

@grappas
Copy link
Collaborator Author

grappas commented Jun 5, 2023

If you allow me I would consider - while optimizing the code - also make code a little bit more readable (make it more modular with inline funtions).

@grappas
Copy link
Collaborator Author

grappas commented Jun 9, 2023

Dude? Are you dead or somethin?

@neural75
Copy link
Owner

Dude? Are you dead or somethin?

I was busy with the job.
The squelch level (is not a calculated noise floor without squelch, isn't it?) could be put in the outside loop to reduce the readings (let's say every 100 steps or so) and we will get the value changed in-flight too, it is not that the squelch changes normally during a single scan. Regarding the full extent of the band to scan, I would not put too much assumptions about other people use cases, it could have been used as a long running monitor of a large portion of a spectrum just to log activities (not to listen), I am not sure.
I will complete this pull request, we can improve it later.

@neural75 neural75 merged commit b4264a6 into master Jun 10, 2023
@grappas
Copy link
Collaborator Author

grappas commented Jun 10, 2023

could be put in the outside loop to reduce the readings

To be perfectly safe I would say separate thread.

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

2 participants