-
Notifications
You must be signed in to change notification settings - Fork 35
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
ERAM Framework #375
base: master
Are you sure you want to change the base?
ERAM Framework #375
Conversation
…into improve-stars
…into improve-stars
…ed track representation (color and brite), etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(A few more comments after another pass through.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More comments about the STARS code. I will try to spend more time with the nas.go
stuff in the next few days.
@@ -534,7 +552,13 @@ func (sp *STARSPane) executeSTARSCommand(cmd string, ctx *panes.Context) (status | |||
if ac := lookupAircraft(cmd); ac == nil { | |||
status.err = ErrSTARSCommandFormat | |||
} else { | |||
sp.initiateTrack(ctx, ac.Callsign) | |||
fp, err := ctx.ControlClient.STARSComputer(ctx.ControlClient.Callsign).GetFlightPlan(ac.Callsign) // TODO: change this so that it's the inputted squawk/ callsign. | |||
if err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is logging the error the right thing to do in this case? i.e. is it expected that there may not always be a flight plan, or does this case mean there's an error in the code elsewhere? If it's valid that there may be no flight plan then I assume a STARS error should be displayed to the user instead?
comp := ctx.ControlClient.STARSComputer(ctx.ControlClient.Callsign) | ||
trk := comp.TrackInformation[callsign] | ||
if trk.TrackOwner == ctx.ControlClient.Callsign { | ||
trk.TempAltitude = alt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this get clobbered within a second when the state update is received from the server? If it's not maintained in stars.AircraftState
(presumably so that other controllers can see it, I think this will need a new RPC call, etc.)
if comp.TrackInformation[callsign] != nil { | ||
// This is a track that is already being tracked | ||
sp.previewAreaOutput = ErrSTARSIllegalTrack.Error() | ||
fmt.Printf("%v: Track %s is already being tracked: %v\n", comp.Identifier, callsign, comp.TrackInformation[callsign]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please delete printf
fmt.Printf("%v: Track %s is already being tracked: %v\n", comp.Identifier, callsign, comp.TrackInformation[callsign]) | ||
return | ||
} | ||
comp.TrackInformation[callsign] = &sim.TrackInformation{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok with limited reading of globals out of the computers structs (similar to what's done elsewhere for e.g., sim.State.Aircraft
, but setting them should probably be done through a method they provide, since the STARS code shouldn't have to potentially worry about any additional housekeeping with changing them.
Also, same question as above about whether it's valid to be setting this value here since I think a sim state update will clobber it.
@@ -2641,7 +2710,12 @@ func (sp *STARSPane) executeSTARSClickedCommand(ctx *panes.Context, cmd string, | |||
if ctrl && shift { | |||
// initiate track, CRC style | |||
status.clear = true | |||
sp.initiateTrack(ctx, ac.Callsign) | |||
fp, err := ctx.ControlClient.STARSComputer(ctx.ControlClient.Callsign).GetFlightPlan(ac.Callsign) | |||
if err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same questions as above about what an error return value implies here.
if ac := ctx.ControlClient.Aircraft[callsign]; comp.TrackInformation[ac.Callsign] == nil && sp.AutoTrackDepartures && ac.DepartureContactController == ctx.ControlClient.Callsign && sim.InAcquisitionArea(ac) { | ||
fp, err := comp.GetFlightPlan(ac.Squawk.String()) | ||
if err != nil { | ||
ctx.Lg.Errorf("GetFlightPlan(%s): %v", ac.Squawk, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The usual error logging question.
@@ -669,6 +759,45 @@ func (sp *STARSPane) drawGhosts(ghosts []*av.GhostAircraft, ctx *panes.Context, | |||
ld.GenerateCommands(cb) | |||
} | |||
|
|||
func (sp *STARSPane) drawUnsupportedTrack(data *sim.UnsupportedTrack, ctx *panes.Context, transforms ScopeTransformations, td *renderer.TextDrawBuilder, | |||
cb *renderer.CommandBuffer) { | |||
// set color and brightness to white/100 for now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stale comments, this line and next?
transforms.LoadWindowViewingMatrices(cb) | ||
td.GenerateCommands(cb) | ||
} | ||
|
||
func (sp *STARSPane) getUnsupportedDatablock(data *sim.UnsupportedTrack, ctx *panes.Context) datablock { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about ut
in place of data
? ("Data" kind of describes anything and everything.)
} | ||
|
||
if fp.AircraftType != "" { | ||
formatDBText(db.field5[1][:], fp.AircraftType, color, false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the other datablock types, fp.AircraftType
needs to be cleaned up a bit just to get the base type without any extras like H/
. Is that also necessary here?
e.g.
actype = trk.FlightPlan.TypeWithoutSuffix()
if strings.Index(actype, "/") == 1 {
actype = actype[2:]
}
if fp.AircraftType != "" { | ||
formatDBText(db.field5[1][:], fp.AircraftType, color, false) | ||
} | ||
if data.SP1 != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the scratchpads, presumably you will want to pad them with spaces to 3 or 4 characters, based on AllowLongScratchpad
in STARSFacilityAdaptation
? After you merge master, check out the updated version of the fmt3
helper function used to do that for the other datablock types; that len([]rune(s))
part is important since the triangle symbol may be in scratchpads and len(s)
of a string gives the length in bytes, not in characters.
New title for improved flight plan processing!
Completed:
STARSComputer
, causing all arrivals to start as PDBs.Aircraft
.ctx.ControlClient.TRACON
forsim.State STARSComputer
calls inpkg/panes/stars
.TODO:
Exit
will occasionally be blank.C
tag.More to come!