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

Integrated grid square calculations with GPSd #435

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

n8jja
Copy link

@n8jja n8jja commented Dec 12, 2023

  1. Moved the position checking inside a Goroutine that runs every 5 minutes.
  2. Created updateGridIfNeeded function, it checks the grid location in config.Locator and updates it if necessary.
  3. Expanded the functionality of the GPSd TPV object to include the grid square as one of its values.
  4. Consolidated all functionality for Maidenhead calculations inside GPSd for simplicity.

…e via GPSd automatically every 5 minutes. The package file contains maidenhead.go and maidenhead_tests.go.

Additionally, added a new go routine to main which will run the update in the background.  The update interval and ability to be tunred on/off have not been implemented yet.
…e via GPSd automatically every 5 minutes. The package file contains maidenhead.go. No test have been written either.

Additionally, added a new go routine to main which will run the update in the background.  The update interval and ability to be tunred on/off have not been implemented yet.
…oved the maidenhead package. Refactored the grid square update routine in main to use the new function and lock the config file while updating.
…oved the maidenhead package. Refactored the grid square update routine in main to use the new function and lock the config file while updating.
…ying the code in main.

* Go routine checks the position every 5 minutes.

* updateGridIfNeeded will check the grid square in config.Locator and update if required.

* GPSd TPV objecrt now includes the gridsquare as one of its values.

* GPSd contains all functionality for maidenhead calculations for simplicity.
…ying the code in main.

* Go routine checks the position every 5 minutes.

* updateGridIfNeeded will check the grid square in config.Locator and update if required.

* GPSd TPV objecrt now includes the gridsquare as one of its values.

* GPSd contains all functionality for maidenhead calculations for simplicity.
@martinhpedersen martinhpedersen changed the base branch from master to develop December 12, 2023 17:34
@martinhpedersen martinhpedersen added this to the v0.16.0 milestone Dec 21, 2023
Comment on lines +31 to +47
GridSquare string `json:"-"`
}

func (t *TPV) CalculateGridSquare() error {
lat, err := t.Lat.Float64()
if err != nil {
return err
}

lon, err := t.Lon.Float64()
if err != nil {
return err
}

point := maidenhead.NewPoint(lat, lon)
t.GridSquare, err = point.GridSquare()
return err
Copy link
Member

Choose a reason for hiding this comment

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

This is no longer needed I believe? conn.GetGridSquare() is not using the TPV's GridSquare, but calculates from LatLon on the fly.

Copy link
Author

Choose a reason for hiding this comment

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

Hi Martin!

You are correct...it is an artifact that I forgot to remove. Thanks!

Copy link
Member

@martinhpedersen martinhpedersen left a comment

Choose a reason for hiding this comment

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

Thanks! I think we're almost there 😄

It appears to me like this PR contains traces of multiple approaches we discussed in the email.

In my mind, this is what we discussed:

Simplest (requires the least changes):

func (p Position) GridSquare() (string, error) {
  return maidenhead.NewPoint(p.Lat, p.Lon).GridSquare()
}

... alternatively add field Position.GridSquare and enrich in func (t TPV) Position() Position ignoring the error.

func (t TPV) Position() Position {
  ...
  // The error is ignored as it's always nil for valid coordinates.
  p.GridSquare, _ := maidenhead.NewPoint(p.Lat, p.Lon).GridSquare()
  return p
}

And finally:

func (c *Conn) NextGridSquare() (string, error) {
  pos, err := c.NextPos()
  if err != nil {
    return "", err
  }
  return maidenhead.NewPoint(lat, lon).GridSquare()
}

The latter is already implemented in this PR, but not used.

Which of the options do you think is best?

In addition I have some concerns regarding the way you persist the updated locator to disk. Please see in-line comments.

internal/gpsd/objects.go Show resolved Hide resolved
internal/gpsd/objects.go Show resolved Hide resolved
@@ -214,6 +215,7 @@ func optionsSet() *pflag.FlagSet {
set.BoolVarP(&fOptions.SendOnly, "send-only", "s", false, "Download inbound messages later, send only.")
set.BoolVarP(&fOptions.RadioOnly, "radio-only", "", false, "Radio Only mode (Winlink Hybrid RMS only).")
set.BoolVar(&fOptions.IgnoreBusy, "ignore-busy", false, "Don't wait for clear channel before connecting to a node.")
set.BoolVar(&fOptions.UpdateGrid, "gridsquare-update", true, "Automatically update the maidenhead grid square from the gpsd daemon.")
Copy link
Member

Choose a reason for hiding this comment

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

We use the term locator in the config. Maybe not the best term, but I think we should use the same term both places sice the two are very much related.

--locator-from-gpsd maybe?

Copy link
Author

Choose a reason for hiding this comment

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

Hi Martin!

Yes, I will change to align with the existing terminology. It makes the most sense to keep things consistent.

Comment on lines +428 to +431
// write config
if err := WriteConfig(config, fOptions.ConfigPath); err != nil {
log.Printf("Unable to write config: %s", err)
}
Copy link
Member

Choose a reason for hiding this comment

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

I don't think rewriting the actual config file is the best idea. It is not a lossless operation, as the user might have defined keys and values that are not part of the JSON spec (e.g. "comments"). Custom formatting will also be lost, and possibly also ordering of aliases etc. It also has the potential of corrupting the file if we for some reason the operation fails in the middle of writing (full disk, power loss++).

I guess it's useful to retain the "last known" position when restarting the app. Otherwise, the user would have to wait the 5 min interval before the locator field is updated again

As I see it, we have two options:

  1. Persist the "last known position" in a separate JSON file, and use that as a initial value (if present).
  2. Block on startup until we get a fresh position from GPSd.

I think the latter might be a better solution, as it guarantees running with --locator-from-gpsd uses a value from GPSd at all times.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks Martin...I agree the latter would be the better option.

Copy link
Author

Choose a reason for hiding this comment

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

After some refreshing on this...won't the gaps.Dial() and conn.NextPos() already blocking?

Comment on lines +397 to +399
for range time.Tick(time.Minute * 5) {
updateGridIfNeeded()
}
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason for polling once per 5 minute? Wouldn't it be even better to continuously keep the locator field up-to-date by subscribing (Watch(true)) and call Next() in an endless loop? 🤔

Copy link
Author

Choose a reason for hiding this comment

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

My thought was that while most people will be mostly static...i.e. once they get the first fix from the GPS, they won't need it anymore. However, for true mobile operation, it might be required to get a new grid square at a regular interval.

I picked 5 minutes at random for testing, but I think long term, this might be something we allow the user to enable/disable and set the interval as needed. If you were in a moving car for example, 5 minutes might be a good interval, but on a sailboat, it might need be only once every few hours.

I'll disable the loop for now.

main.go Show resolved Hide resolved
@@ -235,3 +243,24 @@ func errUnexpected(err error) error {
}
return err
}

// GetGridSquare provides function for getting updated position and calculating the grid square.
func (c *Conn) GetGridSquare() (string, error) {
Copy link
Member

Choose a reason for hiding this comment

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

I believe NextGridSquare() would be a better name for this, to better harmonize with Next() and NextPos() 🙂

Copy link
Author

Choose a reason for hiding this comment

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

Absolutely!

Comment on lines +55 to +60
type GPSdClient interface {
Dial(addr string) (GPSdClient, error)
Watch(enable bool)
Next() (*TPV, error)
}

Copy link
Member

Choose a reason for hiding this comment

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

This is unused? If so, please omit it 😊

Copy link

Choose a reason for hiding this comment

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

I am looking at storing the Locator in the user.json for the multi-user reloader. Like KM4ACK's user swap by config swap
in the PAT Menu 12. GPSd is great and I have one pi feeding 4 pi's locator. only have a 20 minute gap every other month.
in mailbox/CallSign/user.json is (CALLSIGN, Name, TAC), Locator (EL29ER27), WinLink/user (Password), Roll (Owner, Radio Op, Non-Ham), On Duty(00-23), NTS (routing itu-r2, NOAM, USA, TX, STX,) HW ACCESS (Telnet, Wifi2, Wifi5, Ardop, ### Ax25, VaraHF, VaraFM, VaraSat)
Just have to get the Variables loaded at the right time for each Network http connection session or postoffice push/pull.
This lets windows and mac users to get multi-user.... Roll->Multi-user->PostOffice->NTS
Herbert KD5PQJ

@martinhpedersen martinhpedersen removed this from the v0.16.0 milestone Apr 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants