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

Re-add przemienniki.net query source #703

Merged
merged 1 commit into from
Jul 4, 2023
Merged

Re-add przemienniki.net query source #703

merged 1 commit into from
Jul 4, 2023

Conversation

kk7ds
Copy link
Owner

@kk7ds kk7ds commented Jul 4, 2023

This is very rough and lightly tested so far.

Fixes #10700

@kk7ds
Copy link
Owner Author

kk7ds commented Jul 4, 2023

@szporwolik this is a suuuuper fast implementation this morning while eating breakfast. Please test thoroughly ;D

@kk7ds
Copy link
Owner Author

kk7ds commented Jul 4, 2023

Also, if you have better text to put into the "info" blob for this, that would be good. The info on each of these sources is suppose to be a short description, so maybe something like "A european repeater database" or something?

@kk7ds kk7ds force-pushed the feature/10700 branch 2 times, most recently from fc56fbf to e690a4f Compare July 4, 2023 15:53
@kk7ds
Copy link
Owner Author

kk7ds commented Jul 4, 2023

Oops, forgot to commit the new file. I blame breakfast ;)

@szporwolik
Copy link
Contributor

@kk7ds thank your for this quick implementation!

Things that I would suggest to change, prior merging (the minimum functionality):

  • after pulling the repeaters there is "dummy" line at the top : 146.0100 - this could be skipped, seems like a bug
  • switching to digital repeaters (mode = DV) gives a parsing error; I would skip the DV (digital) repeaters for the beginning if this will make the whole feature go online sooner
    2023-07-04 18_33_15-CHIRP (przemienniki net)
  • the distance radius does not work, even when giving all the information still I got all the repeaters in the country (screen) (same comment as with DV, even without this feature getting the list of repeaters from country would be beneficial)
    2023-07-04 18_31_54-CHIRP (przemienniki net)
  • the distance shall have the unit, like "Distance [km]" or something
  • I would add the following information (just a suggestion ;-) ) - "FREE repeater database, which provide most up-to-date information about repeaters in Europe . No account is required."
  • Slovenia "si", Netherlands "nl", Iceland "is", Russia "ru" are missing in the country drop down list comparing to website

@kk7ds
Copy link
Owner Author

kk7ds commented Jul 4, 2023

* after pulling the repeaters there is "dummy" line at the top : 146.0100 - this could be skipped, seems like a bug

Fixed.

* switching to digital repeaters (mode = DV) gives a parsing error; I would skip the DV (digital) repeaters for the beginning if this will make the whole feature go online sooner

Hmm, yeah, DV returns nothing for me in any country (that I tried). I just added this because it was in the other implementation. Perhaps these are all gone, or no longer supported in the database?

* the distance radius does not work, even when giving all the information still I got all the repeaters in the country (screen) (same comment as with DV, even without this feature getting the list of repeaters from country would be beneficial)

Yep, I was passing 'Latitude' instead of 'latitude' (et al) and the API seems to be case-sensitive. I think it's fixed now.

* the distance shall have the unit, like "Distance [km]" or something

This is in the tooltip. I can change this at some point, but it will invalidate all the translation strings for the other dialogs that use the same,.

* I would add the following information (just a suggestion ;-) )  - "FREE repeater database, which provide most up-to-date information about repeaters in Europe . No account is required."

Sounds fine to me.

* Slovenia "si", Netherlands "nl", Iceland "is", Russia "ru" are missing in the country drop down list comparing to website

Done.

Will push an update in a sec. I'm going to be unavailable for the rest of the week, so if you can again test real quick we should be able to get this merged before I go. Thanks!

@szporwolik
Copy link
Contributor

Quickly tested. Functionally I think this is a perfect start!

If possible it would be good the have the query repeaters sorted by name (now it seems a bit random, when coordinates are not given). This is minor feature request.

Great thank you!

@kk7ds
Copy link
Owner Author

kk7ds commented Jul 4, 2023

Okay, cool, thanks for your help. I will merge this soon.

The sort order is whatever is returned from the database server. You think sort by name (not frequency) is best?

@kk7ds
Copy link
Owner Author

kk7ds commented Jul 4, 2023

This update sorts by name if either lat and/or lon is not provided. I think the server sorts by distance when those are provided, so this gives you either server sort order by distance, or name if not. Easy to change to frequency if you think that's better, otherwise this should be good.

This is very rough and lightly tested so far.

Fixes #10700
@kk7ds kk7ds merged commit 37081be into master Jul 4, 2023
6 checks passed
@kk7ds kk7ds deleted the feature/10700 branch July 4, 2023 19:00
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.

2 participants