Address PR review: return structs, validate locations, fix docs

- Refactor Vector/Bullseye/BRA/BRAA from interfaces to public structs
  following idiomatic Go "return structs, accept interfaces"
- Remove redundant conf.Location; use locations.Location directly
- Load locations from file and pass to parser and controller
- Validate reserved location names ("tanker", "bullseye") at startup
- Mark --locations-file as filename flag for shell completion
- Remove unnecessary loc import alias
- Add missing godocs on public symbols
- Fix double comment in vector.go
- Update LOCATIONS.md wording for non-server users

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: dharmab

cmd/skyeye/main.go

@@ -25,10 +25,13 @@ import (
 	"github.com/spf13/pflag"
 	"github.com/spf13/viper"
 
+	"encoding/json"
+
 	"github.com/dharmab/skyeye/internal/application"
 	"github.com/dharmab/skyeye/internal/cli"
 	"github.com/dharmab/skyeye/internal/conf"
 	"github.com/dharmab/skyeye/pkg/coalitions"
+	"github.com/dharmab/skyeye/pkg/locations"
 	"github.com/dharmab/skyeye/pkg/synthesizer/voices"
 	"github.com/ggerganov/whisper.cpp/bindings/go/pkg/whisper"
 )
@@ -154,6 +157,9 @@ func init() {
 	skyeye.Flags().Float64Var(&mandatoryThreatRadiusNM, "mandatory-threat-radius", 25, "Briefed radius for mandatory THREAT calls, in nautical miles")
 	skyeye.Flags().BoolVar(&threatMonitoringRequiresSRS, "threat-monitoring-requires-srs", true, "Require aircraft to be on SRS to receive THREAT calls. Only useful to disable when debugging")
 	skyeye.Flags().StringVar(&locationsFile, "locations-file", "", "Path to file containing additional locations that may be referenced in ALPHA CHECK and VECTOR calls.")
+	if err := skyeye.MarkFlagFilename("locations-file", "json"); err != nil {
+		log.Fatal().Err(err).Msg("failed to mark flag as filename")
+	}
 
 	// Tracing
 	skyeye.Flags().BoolVar(&enableTracing, "enable-tracing", false, "Enable tracing")
@@ -337,6 +343,27 @@ func loadVoiceVolume() float64 {
 	return clamped
 }
 
+func loadLocations() []locations.Location {
+	if locationsFile == "" {
+		return nil
+	}
+	data, err := os.ReadFile(locationsFile)
+	if err != nil {
+		log.Fatal().Err(err).Str("path", locationsFile).Msg("failed to read locations file")
+	}
+	var locs []locations.Location
+	if err := json.Unmarshal(data, &locs); err != nil {
+		log.Fatal().Err(err).Str("path", locationsFile).Msg("failed to parse locations file")
+	}
+	for _, loc := range locs {
+		if err := loc.Validate(); err != nil {
+			log.Fatal().Err(err).Msg("invalid location in locations file")
+		}
+	}
+	log.Info().Int("count", len(locs)).Msg("loaded custom locations")
+	return locs
+}
+
 func preRun(cmd *cobra.Command, _ []string) error {
 	if err := initializeConfig(cmd); err != nil {
 		return fmt.Errorf("failed to initialize config: %w", err)
@@ -384,6 +411,7 @@ func run(_ *cobra.Command, _ []string) {
 	voiceLock := loadLock(voiceLockPath)
 	recognizerLock := loadLock(recognizerLockPath)
 	volume := loadVoiceVolume()
+	locs := loadLocations()
 
 	config := conf.Configuration{
 		ACMIFile:                     acmiFile,
@@ -425,6 +453,7 @@ func run(_ *cobra.Command, _ []string) {
 		GRPCAddress:                  grpcAddress,
 		GRPCAPIKey:                   grpcAPIKey,
 		EnableTerrainDetection:       enableTerrainDetection,
+		Locations:                    locs,
 	}
 
 	log.Info().Msg("starting application")

docs/LOCATIONS.md

@@ -1,6 +1,6 @@
 # Custom Locations
 
-Server administrators can define custom locations which players can reference in ALPHA CHECK and VECTOR requests. You can define useful locations like friendly bases or fix points.
+You can define custom locations which players can reference in ALPHA CHECK and VECTOR requests. You can define useful locations like friendly bases or fix points.
 
 To use this feature, create a `locations.json` file. The content of the file should be a JSON array of objects. Each object should have the following properties:
 

internal/application/app.go

@@ -17,7 +17,6 @@ import (
 	"github.com/dharmab/skyeye/pkg/commands"
 	"github.com/dharmab/skyeye/pkg/composer"
 	"github.com/dharmab/skyeye/pkg/controller"
-	loc "github.com/dharmab/skyeye/pkg/locations"
 	"github.com/dharmab/skyeye/pkg/parser"
 	"github.com/dharmab/skyeye/pkg/radar"
 	"github.com/dharmab/skyeye/pkg/recognizer"
@@ -169,8 +168,13 @@ func NewApplication(config conf.Configuration) (*Application, error) {
 		return nil, fmt.Errorf("failed to construct application: unrecognized recognizer %q", config.Recognizer)
 	}
 
+	locationNames := make([]string, 0)
+	for _, loc := range config.Locations {
+		locationNames = append(locationNames, loc.Names...)
+	}
+
 	log.Info().Msg("constructing request parser")
-	requestParser := parser.New(config.Callsign, []string{}, config.EnableTranscriptionLogging)
+	requestParser := parser.New(config.Callsign, locationNames, config.EnableTranscriptionLogging)
 
 	log.Info().Msg("constructing radar scope")
 
@@ -185,7 +189,7 @@ func NewApplication(config conf.Configuration) (*Application, error) {
 		config.EnableThreatMonitoring,
 		config.ThreatMonitoringInterval,
 		config.ThreatMonitoringRequiresSRS,
-		[]loc.Location{},
+		config.Locations,
 	)
 
 	log.Info().Msg("constructing response composer")

internal/conf/configuration.go

@@ -4,6 +4,7 @@ import (
 	"time"
 
 	"github.com/dharmab/skyeye/pkg/coalitions"
+	"github.com/dharmab/skyeye/pkg/locations"
 	"github.com/dharmab/skyeye/pkg/simpleradio"
 	"github.com/dharmab/skyeye/pkg/synthesizer/voices"
 	"github.com/ggerganov/whisper.cpp/bindings/go/pkg/whisper"
@@ -94,7 +95,7 @@ type Configuration struct {
 	// for debugging.
 	ThreatMonitoringRequiresSRS bool
 	// Locations is a slice of named locations that can be referenced in ALPHA CHECK and VECTOR calls.
-	Locations []*Location
+	Locations []locations.Location
 	// EnableTracing controls whether to publish traces
 	EnableTracing bool
 	// DiscordWebhookID is the ID of the Discord webhook
@@ -108,13 +109,6 @@ type Configuration struct {
 	EnableTerrainDetection bool
 }
 
-type Location struct {
-	// Names of the location
-	Names []string `json:"names"`
-	// Coordinates of the location as a GeoJSON coordinates array with a single member
-	Coordinates [][]float64 `json:"coordinates"`
-}
-
 var DefaultCallsigns = []string{"Sky Eye", "Thunderhead", "Eagle Eye", "Ghost Eye", "Sky Keeper", "Bandog", "Long Caster", "Galaxy"}
 
 var DefaultPictureRadius = 300 * unit.NauticalMile

pkg/brevity/alphacheck.go

@@ -20,5 +20,5 @@ type AlphaCheckResponse struct {
 	// Status is true if the ALPHA CHECK was correlated to an aircraft on frequency, otherwise false.
 	Status bool
 	// Location of the friendly aircraft. If Status is false, this may be nil.
-	Location Bullseye
+	Location *Bullseye
 }

pkg/brevity/braa.go

@@ -11,102 +11,75 @@ import (
 
 // BRAA provides target bearing, range, altitude and aspect relative to a specified friendly aircraft.
 // Reference: ATP 3-52.4 Chapter IV section 4 subsection b.
-type BRAA interface {
+type BRAA struct {
 	BRA
-	// Aspect of the contact.
-	Aspect() Aspect
+	// aspect of the contact.
+	aspect Aspect
 }
 
 // BRA is an abbreviated form of BRAA without aspect.
-type BRA interface {
+type BRA struct {
 	Vector
-	// Altitude of the contact above sea level, rounded to the nearest thousands of feet.
-	Altitude() unit.Length
-	// Altitude STACKS of the contact above sea level, rounded to the nearest thousands of feet.
-	Stacks() []Stack
-}
-
-type bra struct {
-	vector
 	stacks []Stack
 }
 
 // NewBRA creates a new [BRA].
-func NewBRA(b bearings.Bearing, r unit.Length, a ...unit.Length) BRA {
+func NewBRA(b bearings.Bearing, r unit.Length, a ...unit.Length) *BRA {
 	if !b.IsMagnetic() {
 		log.Warn().Stringer("bearing", b).Msg("bearing provided to NewBRA should be magnetic")
 	}
-	return &bra{
-		vector: vector{
+	return &BRA{
+		Vector: Vector{
 			bearing:  b,
 			distance: r,
 		},
 		stacks: Stacks(a...),
 	}
 }
 
-// Altitude implements [BRA.Altitude].
-func (b *bra) Altitude() unit.Length {
+// Altitude returns the highest altitude of the contact above sea level, rounded to the nearest thousands of feet.
+func (b *BRA) Altitude() unit.Length {
 	if len(b.stacks) == 0 {
 		return 0
 	}
 	return spatial.NormalizeAltitude(b.stacks[0].Altitude)
 }
 
-// Stacks implements [BRA.Stacks].
-func (b *bra) Stacks() []Stack {
+// Stacks returns the altitude STACKS of the contact.
+func (b *BRA) Stacks() []Stack {
 	return b.stacks
 }
 
-func (b *bra) String() string {
+func (b *BRA) String() string {
 	s := fmt.Sprintf("BRA %s/%.0f %.0f", b.Bearing(), b.Range().NauticalMiles(), b.Altitude().Feet())
 	if len(b.Stacks()) > 1 {
 		s += fmt.Sprintf(" (%v)", b.Stacks())
 	}
 	return s
 }
 
-type braa struct {
-	bra    BRA
-	aspect Aspect
-}
-
 // NewBRAA creates a new [BRAA].
-func NewBRAA(b bearings.Bearing, r unit.Length, altitudes []unit.Length, aspect Aspect) BRAA {
+func NewBRAA(b bearings.Bearing, r unit.Length, altitudes []unit.Length, aspect Aspect) *BRAA {
 	if !b.IsMagnetic() {
 		log.Warn().Stringer("bearing", b).Msg("bearing provided to NewBRAA should be magnetic")
 	}
-	return &braa{
-		bra:    NewBRA(b, r, altitudes...),
+	return &BRAA{
+		BRA: BRA{
+			Vector: Vector{
+				bearing:  b,
+				distance: r,
+			},
+			stacks: Stacks(altitudes...),
+		},
 		aspect: aspect,
 	}
 }
 
-// Bearing implements [BRA.Bearing].
-func (b *braa) Bearing() bearings.Bearing {
-	return b.bra.Bearing()
-}
-
-// Range implements [BRA.Range].
-func (b *braa) Range() unit.Length {
-	return b.bra.Range()
-}
-
-// Altitude implements [BRA.Altitude].
-func (b *braa) Altitude() unit.Length {
-	return b.bra.Altitude()
-}
-
-// Stacks implements [BRA.Stacks].
-func (b *braa) Stacks() []Stack {
-	return b.bra.Stacks()
-}
-
-// Aspect implements [BRAA.Aspect].
-func (b *braa) Aspect() Aspect {
+// Aspect returns the aspect of the contact.
+func (b *BRAA) Aspect() Aspect {
 	return b.aspect
 }
 
-func (b *braa) String() string {
-	return fmt.Sprintf("%s %s", b.bra, b.Aspect())
+func (b *BRAA) String() string {
+	return fmt.Sprintf("%s %s", &b.BRA, b.Aspect())
 }

pkg/brevity/bullseye.go

@@ -10,38 +10,28 @@ import (
 
 // Bullseye is a magnetic bearing and distance from a reference point called the BULLSEYE.
 // Reference: ATP 3-52.4 Chapter IV section 4 subsection a.
-type Bullseye interface {
-	Bearing() bearings.Bearing
-	Distance() unit.Length
-}
-
-type bullseye struct {
-	vector
+type Bullseye struct {
+	Vector
 }
 
 // NewBullseye creates a new [Bullseye].
-func NewBullseye(bearing bearings.Bearing, distance unit.Length) Bullseye {
+func NewBullseye(bearing bearings.Bearing, distance unit.Length) *Bullseye {
 	if !bearing.IsMagnetic() {
 		log.Warn().Stringer("bearing", bearing).Msg("bearing provided to NewBullseye should be magnetic")
 	}
-	return &bullseye{
-		vector: vector{
+	return &Bullseye{
+		Vector: Vector{
 			bearing:  bearing,
 			distance: distance,
 		},
 	}
 }
 
-// Bearing from the BULLSEYE to the contact, rounded to the nearest degree.
-func (b *bullseye) Bearing() bearings.Bearing {
-	return b.vector.Bearing()
-}
-
 // Distance from the BULLSEYE to the contact, rounded to the nearest nautical mile.
-func (b *bullseye) Distance() unit.Length {
+func (b *Bullseye) Distance() unit.Length {
 	return b.Range()
 }
 
-func (b *bullseye) String() string {
+func (b *Bullseye) String() string {
 	return fmt.Sprintf("%s/%.0f", b.Bearing(), b.Distance().NauticalMiles())
 }

pkg/brevity/declare.go

@@ -52,7 +52,7 @@ type DeclareRequest struct {
 	// were providing Bullseye or BRAA coordinates.
 	IsAmbiguous bool
 	// Bullseye of the contact, if provided using Bullseye.
-	Bullseye Bullseye
+	Bullseye *Bullseye
 	// Bearing of the contact, if provided using BRAA.
 	Bearing bearings.Bearing
 	/// Range to the contact, if provided using BRAA.
@@ -92,8 +92,7 @@ type DeclareResponse struct {
 	// because the friendly aircraft did not provide coordinates for the
 	// contact.
 	Sour bool
-	// If readback is not nil, the controller should read back the coordinate
-	// in the response.
+	// Readback is the bullseye coordinate to read back in the response. May be nil.
 	Readback *Bullseye
 	// Declaration of the contact.
 	Declaration Declaration

pkg/brevity/group.go

@@ -19,7 +19,7 @@ type Group interface {
 	// Contacts is the number of contacts in the group.
 	Contacts() int
 	// Bullseye is the location of the group. This may be nil for BOGEY DOPE, SNAPLOCK, and THREAT calls.
-	Bullseye() Bullseye
+	Bullseye() *Bullseye
 	// Altitude is the group's highest altitude. This may be zero for BOGEY DOPE, SNAPLOCK, and THREAT calls.
 	Altitude() unit.Length
 	// Stacks are the group's altitude STACKS, ordered from highest to lowest in intervals of at least 10,000 feet.
@@ -30,7 +30,7 @@ type Group interface {
 	// Aspect is the group's aspect angle relative to another aircraft. This may be nil for BOGEY DOPE, SNAPLOCK, SPIKED, STROBE and some THREAT calls.
 	Aspect() Aspect
 	// BRAA is an alternate format for the group's location. This is nil except for BOGEY DOPE, SNAPLOCK, SPIKED, STROBE and some THREAT calls.
-	BRAA() BRAA
+	BRAA() *BRAA
 	// Declaration of the group's friend or foe status.
 	Declaration() Declaration
 	// SetDeclaration sets the group's friend or foe status.

pkg/brevity/snaplock.go

@@ -9,7 +9,7 @@ type SnaplockRequest struct {
 	// Callsign of the friendly aircraft requesting the SNAPLOCK.
 	Callsign string
 	// BRA is the location of the contact.
-	BRA BRA
+	BRA *BRA
 }
 
 func (r SnaplockRequest) String() string {