From c17799719df0af9f92df6a970d8eb995b4ca1d77 Mon Sep 17 00:00:00 2001 From: Patrick Fairbank Date: Sat, 14 Oct 2023 16:05:30 -0700 Subject: [PATCH] Refactor to slightly improve memory safety of TeamWifiStatus. --- field/arena.go | 78 +++++++++++++++++++----------- field/arena_notifiers.go | 24 ++++----- network/access_point.go | 21 +++++--- network/access_point_test.go | 32 +++++++----- static/js/field_monitor_display.js | 8 +-- static/js/match_play.js | 2 +- 6 files changed, 98 insertions(+), 67 deletions(-) diff --git a/field/arena.go b/field/arena.go index a8d59bd5..56744a4c 100644 --- a/field/arena.go +++ b/field/arena.go @@ -84,12 +84,13 @@ type Arena struct { } type AllianceStation struct { - DsConn *DriverStationConnection - Ethernet bool - Astop bool - Estop bool - Bypass bool - Team *model.Team + DsConn *DriverStationConnection + Ethernet bool + Astop bool + Estop bool + Bypass bool + Team *model.Team + WifiStatus network.TeamWifiStatus } // Creates the arena and sets it to its initial state. @@ -98,16 +99,6 @@ func NewArena(dbPath string) (*Arena, error) { arena.configureNotifiers() arena.Plc = new(plc.ModbusPlc) - var err error - arena.Database, err = model.OpenDatabase(dbPath) - if err != nil { - return nil, err - } - err = arena.LoadSettings() - if err != nil { - return nil, err - } - arena.AllianceStations = make(map[string]*AllianceStation) arena.AllianceStations["R1"] = new(AllianceStation) arena.AllianceStations["R2"] = new(AllianceStation) @@ -118,6 +109,16 @@ func NewArena(dbPath string) (*Arena, error) { arena.Displays = make(map[string]*Display) + var err error + arena.Database, err = model.OpenDatabase(dbPath) + if err != nil { + return nil, err + } + err = arena.LoadSettings() + if err != nil { + return nil, err + } + arena.ScoringPanelRegistry.initialize() // Load empty match as current. @@ -144,6 +145,35 @@ func (arena *Arena) LoadSettings() error { arena.EventSettings = settings // Initialize the components that depend on settings. + var accessPoint1WifiStatuses, accessPoint2WifiStatuses [6]*network.TeamWifiStatus + if arena.EventSettings.Ap2TeamChannel == 0 { + accessPoint1WifiStatuses = [6]*network.TeamWifiStatus{ + &arena.AllianceStations["R1"].WifiStatus, + &arena.AllianceStations["R2"].WifiStatus, + &arena.AllianceStations["R3"].WifiStatus, + &arena.AllianceStations["B1"].WifiStatus, + &arena.AllianceStations["B2"].WifiStatus, + &arena.AllianceStations["B3"].WifiStatus, + } + } else { + accessPoint1WifiStatuses = [6]*network.TeamWifiStatus{ + &arena.AllianceStations["R1"].WifiStatus, + &arena.AllianceStations["R2"].WifiStatus, + &arena.AllianceStations["R3"].WifiStatus, + nil, + nil, + nil, + } + accessPoint2WifiStatuses = [6]*network.TeamWifiStatus{ + nil, + nil, + nil, + &arena.AllianceStations["B1"].WifiStatus, + &arena.AllianceStations["B2"].WifiStatus, + &arena.AllianceStations["B3"].WifiStatus, + } + } + arena.accessPoint.SetSettings( settings.ApType == "vivid", settings.ApAddress, @@ -151,6 +181,7 @@ func (arena *Arena) LoadSettings() error { settings.ApPassword, settings.ApTeamChannel, settings.NetworkSecurityEnabled, + accessPoint1WifiStatuses, ) arena.accessPoint2.SetSettings( settings.ApType == "vivid", @@ -159,6 +190,7 @@ func (arena *Arena) LoadSettings() error { settings.Ap2Password, settings.Ap2TeamChannel, settings.NetworkSecurityEnabled, + accessPoint2WifiStatuses, ) arena.networkSwitch = network.NewSwitch(settings.SwitchAddress, settings.SwitchPassword) arena.Plc.SetAddress(settings.PlcAddress) @@ -370,20 +402,10 @@ func (arena *Arena) StartMatch() error { } arena.updateCycleTime(arena.CurrentMatch.StartedAt) - // Convert AP team wifi network status array to a map by station for ease of client use. - teamWifiStatuses := make(map[string]*network.TeamWifiStatus) - for i, station := range []string{"R1", "R2", "R3", "B1", "B2", "B3"} { - if arena.EventSettings.Ap2TeamChannel == 0 || i < 3 { - teamWifiStatuses[station] = &arena.accessPoint.TeamWifiStatuses[i] - } else { - teamWifiStatuses[station] = &arena.accessPoint2.TeamWifiStatuses[i] - } - } - // Save the missed packet count to subtract it from the running count. - for station, allianceStation := range arena.AllianceStations { + for _, allianceStation := range arena.AllianceStations { if allianceStation.DsConn != nil { - err = allianceStation.DsConn.signalMatchStart(arena.CurrentMatch, teamWifiStatuses[station]) + err = allianceStation.DsConn.signalMatchStart(arena.CurrentMatch, &allianceStation.WifiStatus) if err != nil { log.Println(err) } diff --git a/field/arena_notifiers.go b/field/arena_notifiers.go index 48df2207..342c6231 100644 --- a/field/arena_notifiers.go +++ b/field/arena_notifiers.go @@ -8,7 +8,6 @@ package field import ( "github.com/Team254/cheesy-arena/game" "github.com/Team254/cheesy-arena/model" - "github.com/Team254/cheesy-arena/network" "github.com/Team254/cheesy-arena/playoff" "github.com/Team254/cheesy-arena/websocket" "strconv" @@ -73,28 +72,23 @@ func (arena *Arena) generateAllianceStationDisplayModeMessage() any { } func (arena *Arena) generateArenaStatusMessage() any { - // Convert AP team wifi network status array to a map by station for ease of client use. - teamWifiStatuses := make(map[string]network.TeamWifiStatus) - for i, station := range []string{"R1", "R2", "R3", "B1", "B2", "B3"} { - if arena.EventSettings.Ap2TeamChannel == 0 || i < 3 { - teamWifiStatuses[station] = arena.accessPoint.TeamWifiStatuses[i] - } else { - teamWifiStatuses[station] = arena.accessPoint2.TeamWifiStatuses[i] - } - } - return &struct { MatchId int AllianceStations map[string]*AllianceStation - TeamWifiStatuses map[string]network.TeamWifiStatus MatchState CanStartMatch bool PlcIsHealthy bool FieldEstop bool PlcArmorBlockStatuses map[string]bool - }{arena.CurrentMatch.Id, arena.AllianceStations, teamWifiStatuses, arena.MatchState, - arena.checkCanStartMatch() == nil, arena.Plc.IsHealthy(), arena.Plc.GetFieldEstop(), - arena.Plc.GetArmorBlockStatuses()} + }{ + arena.CurrentMatch.Id, + arena.AllianceStations, + arena.MatchState, + arena.checkCanStartMatch() == nil, + arena.Plc.IsHealthy(), + arena.Plc.GetFieldEstop(), + arena.Plc.GetArmorBlockStatuses(), + } } func (arena *Arena) generateAudienceDisplayModeMessage() any { diff --git a/network/access_point.go b/network/access_point.go index c8482db1..7e448592 100644 --- a/network/access_point.go +++ b/network/access_point.go @@ -38,7 +38,7 @@ type AccessPoint struct { teamChannel int networkSecurityEnabled bool configRequestChan chan [6]*model.Team - TeamWifiStatuses [6]TeamWifiStatus + TeamWifiStatuses [6]*TeamWifiStatus initialStatusesFetched bool } @@ -57,7 +57,11 @@ type sshOutput struct { } func (ap *AccessPoint) SetSettings( - isVividType bool, address, username, password string, teamChannel int, networkSecurityEnabled bool, + isVividType bool, + address, username, password string, + teamChannel int, + networkSecurityEnabled bool, + wifiStatuses [6]*TeamWifiStatus, ) { ap.isVividType = isVividType ap.address = address @@ -65,6 +69,7 @@ func (ap *AccessPoint) SetSettings( ap.password = password ap.teamChannel = teamChannel ap.networkSecurityEnabled = networkSecurityEnabled + ap.TeamWifiStatuses = wifiStatuses // Create config channel the first time this method is called. if ap.configRequestChan == nil { @@ -207,7 +212,7 @@ func (ap *AccessPoint) updateTeamWifiStatuses() error { output, err := ap.runCommand("iwinfo") if err == nil { logWifiInfo(output) - err = decodeWifiInfo(output, ap.TeamWifiStatuses[:]) + err = ap.decodeWifiInfo(output) } if err != nil { @@ -303,7 +308,7 @@ func logWifiInfo(wifiInfo string) { } // Parses the given output from the "iwinfo" command on the AP and updates the given status structure with the result. -func decodeWifiInfo(wifiInfo string, statuses []TeamWifiStatus) error { +func (ap *AccessPoint) decodeWifiInfo(wifiInfo string) error { ssidRe := regexp.MustCompile("ESSID: \"([-\\w ]*)\"") ssids := ssidRe.FindAllStringSubmatch(wifiInfo, -1) @@ -312,9 +317,11 @@ func decodeWifiInfo(wifiInfo string, statuses []TeamWifiStatus) error { return fmt.Errorf("Could not parse wifi info; expected 6 team networks, got %d.", len(ssids)) } - for i := range statuses { - ssid := ssids[i][1] - statuses[i].TeamId, _ = strconv.Atoi(ssid) // Any non-numeric SSIDs will be represented by a zero. + for i, wifiStatus := range ap.TeamWifiStatuses { + if wifiStatus != nil { + ssid := ssids[i][1] + wifiStatus.TeamId, _ = strconv.Atoi(ssid) // Any non-numeric SSIDs will be represented by a zero. + } } return nil diff --git a/network/access_point_test.go b/network/access_point_test.go index 0d1943ed..948510a4 100644 --- a/network/access_point_test.go +++ b/network/access_point_test.go @@ -131,49 +131,57 @@ func TestGenerateTeamAccessPointConfigForVividHosting(t *testing.T) { } func TestDecodeWifiInfo(t *testing.T) { - var statuses [6]TeamWifiStatus + statuses := [6]*TeamWifiStatus{ + nil, + &TeamWifiStatus{}, + &TeamWifiStatus{}, + &TeamWifiStatus{}, + nil, + &TeamWifiStatus{}, + } + ap := AccessPoint{isVividType: true, TeamWifiStatuses: statuses} // Test with zero team networks configured. output, err := ioutil.ReadFile("testdata/iwinfo_0_teams.txt") if assert.Nil(t, err) { - assert.Nil(t, decodeWifiInfo(string(output), statuses[:])) - assert.Equal(t, 0, statuses[0].TeamId) + assert.Nil(t, ap.decodeWifiInfo(string(output))) + assert.Nil(t, statuses[0]) assert.Equal(t, 0, statuses[1].TeamId) assert.Equal(t, 0, statuses[2].TeamId) assert.Equal(t, 0, statuses[3].TeamId) - assert.Equal(t, 0, statuses[4].TeamId) + assert.Nil(t, statuses[4]) assert.Equal(t, 0, statuses[5].TeamId) } // Test with two team networks configured. output, err = ioutil.ReadFile("testdata/iwinfo_2_teams.txt") if assert.Nil(t, err) { - assert.Nil(t, decodeWifiInfo(string(output), statuses[:])) - assert.Equal(t, 0, statuses[0].TeamId) + assert.Nil(t, ap.decodeWifiInfo(string(output))) + assert.Nil(t, statuses[0]) assert.Equal(t, 2471, statuses[1].TeamId) assert.Equal(t, 0, statuses[2].TeamId) assert.Equal(t, 254, statuses[3].TeamId) - assert.Equal(t, 0, statuses[4].TeamId) + assert.Nil(t, statuses[4]) assert.Equal(t, 0, statuses[5].TeamId) } // Test with six team networks configured. output, err = ioutil.ReadFile("testdata/iwinfo_6_teams.txt") if assert.Nil(t, err) { - assert.Nil(t, decodeWifiInfo(string(output), statuses[:])) - assert.Equal(t, 254, statuses[0].TeamId) + assert.Nil(t, ap.decodeWifiInfo(string(output))) + assert.Nil(t, statuses[0]) assert.Equal(t, 1678, statuses[1].TeamId) assert.Equal(t, 2910, statuses[2].TeamId) assert.Equal(t, 604, statuses[3].TeamId) - assert.Equal(t, 8, statuses[4].TeamId) + assert.Nil(t, statuses[4]) assert.Equal(t, 2471, statuses[5].TeamId) } // Test with invalid input. - assert.NotNil(t, decodeWifiInfo("", statuses[:])) + assert.NotNil(t, ap.decodeWifiInfo("")) output, err = ioutil.ReadFile("testdata/iwinfo_invalid.txt") if assert.Nil(t, err) { - assert.NotNil(t, decodeWifiInfo(string(output), statuses[:])) + assert.NotNil(t, ap.decodeWifiInfo(string(output))) } } diff --git a/static/js/field_monitor_display.js b/static/js/field_monitor_display.js index 4ac70bff..e81ccef5 100644 --- a/static/js/field_monitor_display.js +++ b/static/js/field_monitor_display.js @@ -18,7 +18,7 @@ var handleArenaStatus = function(data) { } else if (currentMatchId !== data.MatchId) { location.reload(); } - + $.each(data.AllianceStations, function(station, stationStatus) { // Select the DOM elements corresponding to the team station. var teamElementPrefix; @@ -75,7 +75,7 @@ var handleArenaStatus = function(data) { teamEthernetElement.text("ETH"); } - var wifiStatus = data.TeamWifiStatuses[station]; + const wifiStatus = stationStatus.WifiStatus; teamRadioTextElement.text(wifiStatus.TeamId); if (stationStatus.DsConn) { @@ -205,7 +205,7 @@ $(function() { redSide = "left"; blueSide = "right"; } - + //Read if display to be used in a Driver Station, ignore FTA flag if so. var driverStation = urlParams.get("ds"); if (driverStation === "true") { @@ -215,7 +215,7 @@ $(function() { $(".fta-dependent").attr("data-fta", urlParams.get("fta")); $(".ds-dependent").attr("data-ds", driverStation); } - + $(".reversible-left").attr("data-reversed", reversed); $(".reversible-right").attr("data-reversed", reversed); diff --git a/static/js/match_play.js b/static/js/match_play.js index 95450f58..f1c9f847 100644 --- a/static/js/match_play.js +++ b/static/js/match_play.js @@ -109,7 +109,7 @@ const getTeamNumber = function(station) { const handleArenaStatus = function(data) { // Update the team status view. $.each(data.AllianceStations, function(station, stationStatus) { - const wifiStatus = data.TeamWifiStatuses[station]; + const wifiStatus = stationStatus.WifiStatus; $("#status" + station + " .radio-status").text(wifiStatus.TeamId); if (stationStatus.DsConn) {