From a397d6ef6d920c167d25064480c06020a4a1d09c Mon Sep 17 00:00:00 2001 From: Reese Norris Date: Fri, 17 Jul 2026 15:54:39 -0400 Subject: [PATCH] fix: address review feedback for sweatbox engine skeleton Reject NaN/Inf geometry on add; clear Ident on ss; combined runway name defaults to RwyA; simplify rules switch; test SetMaxAircraft below current count. --- internal/sweatbox/dispatch.go | 53 +++++++++------- internal/sweatbox/engine.go | 9 ++- internal/sweatbox/engine_test.go | 105 +++++++++++++++++++++++++++++-- 3 files changed, 137 insertions(+), 30 deletions(-) diff --git a/internal/sweatbox/dispatch.go b/internal/sweatbox/dispatch.go index f7439ed..d5c4c93 100644 --- a/internal/sweatbox/dispatch.go +++ b/internal/sweatbox/dispatch.go @@ -2,6 +2,7 @@ package sweatbox import ( "fmt" + "math" "strconv" "strings" ) @@ -129,6 +130,10 @@ func (e *Engine) cmdXPDRLocked(target, mode string) CommandResult { return CommandResult{OK: false, Message: errMsg} } ac.XPDRMode = mode + // Standby and ident are contradictory on the wire; clear flash on ss. + if mode == XPDRModeStandby { + ac.Ident = false + } return CommandResult{OK: true} } @@ -177,19 +182,9 @@ func (e *Engine) cmdAddLocked(args []string) CommandResult { switch rules { case RulesVFR, RulesIFR, RulesDVFR, RulesSVFR: - // ok — also accept lowercase already uppercased + // ok (args already upper-cased) default: - // accept single-letter v/i/d/s - if len(rules) == 1 { - switch rules { - case "V", "I", "D", "S": - // ok - default: - return CommandResult{OK: false, Message: addUsage} - } - } else { - return CommandResult{OK: false, Message: addUsage} - } + return CommandResult{OK: false, Message: addUsage} } switch weight { case WeightSmall, WeightSmallP, WeightLarge, WeightHeavy: @@ -231,14 +226,10 @@ func (e *Engine) cmdAddLocked(args []string) CommandResult { if len(args) < 6 { return CommandResult{OK: false, Message: addUsage} } - bearingStr := loc[1:] - bearing, err := strconv.ParseFloat(bearingStr, 64) - if err != nil { - return CommandResult{OK: false, Message: addUsage} - } - distNM, err1 := strconv.ParseFloat(args[4], 64) - alt, err2 := strconv.ParseFloat(args[5], 64) - if err1 != nil || err2 != nil || distNM < 0 { + bearing, ok1 := parseFiniteFloat(loc[1:]) + distNM, ok2 := parseFiniteFloat(args[4]) + alt, ok3 := parseFiniteFloat(args[5]) + if !ok1 || !ok2 || !ok3 || distNM < 0 { return CommandResult{OK: false, Message: addUsage} } typeTok, errMsg := optionalType(args[6:]) @@ -254,8 +245,8 @@ func (e *Engine) cmdAddLocked(args []string) CommandResult { return CommandResult{OK: false, Message: addUsage} } rwy := strings.ToUpper(loc) - distNM, err := strconv.ParseFloat(args[4], 64) - if err != nil || distNM < 0 { + distNM, ok := parseFiniteFloat(args[4]) + if !ok || distNM < 0 { return CommandResult{OK: false, Message: addUsage} } typeTok, errMsg := optionalType(args[5:]) @@ -286,6 +277,15 @@ func (e *Engine) cmdAddLocked(args []string) CommandResult { return CommandResult{OK: true, Added: []AircraftSnapshot{ac.snapshot()}} } +// parseFiniteFloat parses a float that must be finite (rejects NaN/±Inf). +func parseFiniteFloat(s string) (float64, bool) { + v, err := strconv.ParseFloat(strings.TrimSpace(s), 64) + if err != nil || math.IsNaN(v) || math.IsInf(v, 0) { + return 0, false + } + return v, true +} + func optionalType(args []string) (string, string) { if len(args) == 0 { return "", "" @@ -367,6 +367,11 @@ func (e *Engine) placeOnApproachLocked(ac *SimAircraft, rwy string, distNM float if !ok { return "Runway/taxiway not found in airport file." } + // Record the resolved end designator (combined "33/15" → RwyA). + landEnd := rwy + if rwy == s.Name || rwy == s.RwyA+"/"+s.RwyB { + landEnd = s.RwyA + } // Place along final: opposite of landing heading from threshold. finalBearing := normalizeHeading(hdg + 180) lat, lon := destinationPoint(thr.Lat, thr.Lon, finalBearing, distNM*metersPerNM) @@ -376,7 +381,7 @@ func (e *Engine) placeOnApproachLocked(ac *SimAircraft, rwy string, distNM float ac.Heading = hdg ac.Speed = defaultApproachSpeed(ac.Engine) ac.Status = StatusOnApproach - ac.LandingRunway = rwy - ac.Instruction = "Approach runway " + rwy + ac.LandingRunway = landEnd + ac.Instruction = "Approach runway " + landEnd return "" } diff --git a/internal/sweatbox/engine.go b/internal/sweatbox/engine.go index 10c2337..5e50cab 100644 --- a/internal/sweatbox/engine.go +++ b/internal/sweatbox/engine.go @@ -460,13 +460,18 @@ func initialBearingDeg(lat1, lon1, lat2, lon2 float64) float64 { } // runwayThreshold returns the threshold point and landing heading for a runway -// end designator (e.g. "19" or "33"). Landing heading is along the runway -// centerline from the threshold toward the far end. +// end designator (e.g. "19" or "33"). Combined names ("19/1") default to RwyA. +// Landing heading is along the runway centerline from the threshold toward the +// far end. func runwayThreshold(s *Surface, end string) (threshold Point, hdg float64, ok bool) { if s == nil || s.Kind != SurfaceRunway || len(s.Points) < 2 { return Point{}, 0, false } end = strings.ToUpper(strings.TrimSpace(end)) + // Combined "A/B" (or full surface Name) → approach end A by default. + if end == s.Name || end == s.RwyA+"/"+s.RwyB { + end = s.RwyA + } // Points are ordered RwyA → RwyB in .apt files. var thr, far Point var dispFt float64 diff --git a/internal/sweatbox/engine_test.go b/internal/sweatbox/engine_test.go index b479091..1305165 100644 --- a/internal/sweatbox/engine_test.go +++ b/internal/sweatbox/engine_test.go @@ -430,6 +430,14 @@ func TestCommand_AddValidation(t *testing.T) { {"add i l j -270 10", false, "Missing parameters"}, // bearing needs alt {"add i l j 33 abc", false, "Missing parameters"}, {"add i l j 33 10 EXTRA JUNK", false, "Missing parameters"}, + // Non-finite geometry args (Issue 1) + {"add i l j 33 NaN", false, "Missing parameters"}, + {"add i l j 33 +Inf", false, "Missing parameters"}, + {"add i l j 33 -Inf", false, "Missing parameters"}, + {"add v s p -270 10 NaN", false, "Missing parameters"}, + {"add v s p -NaN 10 2500", false, "Missing parameters"}, + {"add v s p -270 Inf 2500", false, "Missing parameters"}, + {"add v s p -+Inf 10 2500", false, "Missing parameters"}, } for _, tc := range cases { r := e.CommandLine(tc.line) @@ -469,6 +477,90 @@ func TestCommand_AddMaxAircraft(t *testing.T) { } } +// Issue 5: SetMaxAircraft reduced below current count keeps existing AC +// and rejects further adds until under the new cap. +func TestSetMaxAircraft_BelowCurrentCount(t *testing.T) { + e := loadKBTVEngine(t) + var css []string + for i := 0; i < 3; i++ { + r := e.CommandLine("add v s p @GA1") + if !r.OK { + t.Fatalf("add %d: %s", i, r.Message) + } + css = append(css, r.Added[0].Callsign) + } + if e.Count() != 3 { + t.Fatalf("count = %d", e.Count()) + } + e.SetMaxAircraft(1) + if e.Settings().MaxAircraft != 1 { + t.Fatalf("max = %d", e.Settings().MaxAircraft) + } + if e.Count() != 3 { + t.Fatalf("existing aircraft must be kept: count=%d", e.Count()) + } + r := e.CommandLine("add v s p @GA2") + if r.OK || !contains(r.Message, "Maximum") { + t.Fatalf("add while over cap: %+v", r) + } + // Free slots until under cap, then add succeeds. + if !e.Delete(css[0]) || !e.Delete(css[1]) { + t.Fatal("delete") + } + if e.Count() != 1 { + t.Fatalf("count after del = %d", e.Count()) + } + r = e.CommandLine("add v s p @GA2") + if r.OK { + t.Fatal("still at cap (1); further add should fail") + } + e.Delete(css[2]) + r = e.CommandLine("add v s p @GA2") + if !r.OK { + t.Fatalf("add under cap: %s", r.Message) + } + if e.Count() != 1 { + t.Fatalf("count = %d", e.Count()) + } +} + +func (e *Engine) mustGet(t *testing.T, cs string) AircraftSnapshot { + t.Helper() + ac, ok := e.Get(cs) + if !ok { + t.Fatalf("missing %s", cs) + } + return ac +} + +func TestCommand_AddCombinedRunwayName(t *testing.T) { + // Issue 3: combined designator "33/15" should place on RwyA approach. + e := loadKBTVEngine(t) + r := e.CommandLine("add i l j 33/15 8") + if !r.OK { + t.Fatalf("add combined: %s", r.Message) + } + ac := r.Added[0] + if ac.Status != StatusOnApproach { + t.Errorf("status = %s", ac.Status) + } + if ac.LandingRunway != "33" { + t.Errorf("LandingRunway = %s, want 33 (RwyA)", ac.LandingRunway) + } + // Same geometry as end designator "33". + r2 := e.CommandLine("add i l j 33 8") + if !r2.OK { + t.Fatal(r2.Message) + } + ac2 := r2.Added[0] + if math.Abs(ac.Lat-ac2.Lat) > 1e-6 || math.Abs(ac.Lon-ac2.Lon) > 1e-6 { + t.Errorf("combined vs end mismatch: (%v,%v) vs (%v,%v)", ac.Lat, ac.Lon, ac2.Lat, ac2.Lon) + } + if math.Abs(ac.Heading-ac2.Heading) > 1e-6 { + t.Errorf("hdg %v vs %v", ac.Heading, ac2.Heading) + } +} + func TestCommand_DelPosSqId(t *testing.T) { e := loadKBTVEngine(t) r := e.CommandLine("add v s p @GA1") @@ -511,6 +603,7 @@ func TestCommand_DelPosSqId(t *testing.T) { t.Errorf("sqi: %+v", ac) } + // ss must clear Ident after id/sqi (Issue 2). r = e.Command(cs, "ss") if !r.OK { t.Fatal(r.Message) @@ -519,6 +612,9 @@ func TestCommand_DelPosSqId(t *testing.T) { if ac.XPDRMode != XPDRModeStandby { t.Errorf("mode = %s", ac.XPDRMode) } + if ac.Ident { + t.Error("ss should clear Ident") + } r = e.Command(cs, "sn") if !r.OK { t.Fatal(r.Message) @@ -528,10 +624,6 @@ func TestCommand_DelPosSqId(t *testing.T) { t.Errorf("mode = %s", ac.XPDRMode) } - // Clear ident then set via id. - e.mu.Lock() - e.aircraft[cs].Ident = false - e.mu.Unlock() r = e.Command(cs, "id") if !r.OK { t.Fatal(r.Message) @@ -540,6 +632,11 @@ func TestCommand_DelPosSqId(t *testing.T) { if !ac.Ident { t.Error("ident not set") } + // ss after id clears ident again + r = e.Command(cs, "ss") + if !r.OK || e.mustGet(t, cs).Ident { + t.Fatalf("ss after id: ok=%v ident=%v", r.OK, e.mustGet(t, cs).Ident) + } // Bad squawk. r = e.Command(cs, "sq abcd")