diff --git a/field/arena_notifiers.go b/field/arena_notifiers.go index fa10614..5e530bb 100644 --- a/field/arena_notifiers.go +++ b/field/arena_notifiers.go @@ -33,11 +33,6 @@ type ArenaNotifiers struct { ControlPanelColorNotifier *websocket.Notifier } -type DisplayConfigurationMessage struct { - Displays map[string]*Display - DisplayUrls map[string]string -} - type MatchTimeMessage struct { MatchState MatchTimeSec int @@ -107,16 +102,14 @@ func (arena *Arena) generateAudienceDisplayModeMessage() interface{} { } func (arena *Arena) generateDisplayConfigurationMessage() interface{} { + // Notify() for this notifier must always called from a method that has a lock on the display mutex. // Make a copy of the map to avoid potential data races; otherwise the same map would get iterated through as it is - // serialized to JSON. - displaysCopy := make(map[string]*Display) - displayUrls := make(map[string]string) + // serialized to JSON, outside the mutex lock. + displaysCopy := make(map[string]Display) for displayId, display := range arena.Displays { - displaysCopy[displayId] = display - displayUrls[displayId] = display.ToUrl() + displaysCopy[displayId] = *display } - - return &DisplayConfigurationMessage{displaysCopy, displayUrls} + return displaysCopy } func (arena *Arena) generateEventStatusMessage() interface{} { diff --git a/field/display.go b/field/display.go index acca26a..0e100c0 100644 --- a/field/display.go +++ b/field/display.go @@ -7,6 +7,7 @@ package field import ( "fmt" + "github.com/Team254/cheesy-arena/websocket" "net/url" "reflect" "sort" @@ -60,64 +61,69 @@ var displayTypePaths = map[DisplayType]string{ var displayRegistryMutex sync.Mutex type Display struct { - Id string - Nickname string - Type DisplayType - Configuration map[string]string - IpAddress string - ConnectionCount int - lastConnectedTime time.Time + DisplayConfiguration DisplayConfiguration + IpAddress string + ConnectionCount int + Notifier *websocket.Notifier + lastConnectedTime time.Time +} + +type DisplayConfiguration struct { + Id string + Nickname string + Type DisplayType + Configuration map[string]string } // Parses the given display URL path and query string to extract the configuration. -func DisplayFromUrl(path string, query map[string][]string) (*Display, error) { +func DisplayFromUrl(path string, query map[string][]string) (*DisplayConfiguration, error) { if _, ok := query["displayId"]; !ok { return nil, fmt.Errorf("Display ID not present in request.") } - var display Display - display.Id = query["displayId"][0] + var displayConfig DisplayConfiguration + displayConfig.Id = query["displayId"][0] if nickname, ok := query["nickname"]; ok { - display.Nickname, _ = url.QueryUnescape(nickname[0]) + displayConfig.Nickname, _ = url.QueryUnescape(nickname[0]) } // Determine type from the websocket connection URL. This way of doing it isn't super efficient, but it's not really // a concern since it should happen relatively infrequently. for displayType, displayPath := range displayTypePaths { if path == displayPath+"/websocket" { - display.Type = displayType + displayConfig.Type = displayType break } } - if display.Type == InvalidDisplay { + if displayConfig.Type == InvalidDisplay { return nil, fmt.Errorf("Could not determine display type from path %s.", path) } // Put any remaining query parameters into the per-type configuration map. - display.Configuration = make(map[string]string) + displayConfig.Configuration = make(map[string]string) for key, value := range query { if key != "displayId" && key != "nickname" { - display.Configuration[key], _ = url.QueryUnescape(value[0]) + displayConfig.Configuration[key], _ = url.QueryUnescape(value[0]) } } - return &display, nil + return &displayConfig, nil } // Returns the URL string for the given display that includes all of its configuration parameters. func (display *Display) ToUrl() string { var builder strings.Builder - builder.WriteString(displayTypePaths[display.Type]) + builder.WriteString(displayTypePaths[display.DisplayConfiguration.Type]) builder.WriteString("?displayId=") - builder.WriteString(url.QueryEscape(display.Id)) - if display.Nickname != "" { + builder.WriteString(url.QueryEscape(display.DisplayConfiguration.Id)) + if display.DisplayConfiguration.Nickname != "" { builder.WriteString("&nickname=") - builder.WriteString(url.QueryEscape(display.Nickname)) + builder.WriteString(url.QueryEscape(display.DisplayConfiguration.Nickname)) } // Sort the keys so that the URL generated is deterministic. var keys []string - for key := range display.Configuration { + for key := range display.DisplayConfiguration.Configuration { keys = append(keys, key) } sort.Strings(keys) @@ -125,11 +131,15 @@ func (display *Display) ToUrl() string { builder.WriteString("&") builder.WriteString(url.QueryEscape(key)) builder.WriteString("=") - builder.WriteString(url.QueryEscape(display.Configuration[key])) + builder.WriteString(url.QueryEscape(display.DisplayConfiguration.Configuration[key])) } return builder.String() } +func (display *Display) generateDisplayConfigurationMessage() interface{} { + return display.ToUrl() +} + // Returns an unused ID that can be used for a new display. func (arena *Arena) NextDisplayId() string { displayRegistryMutex.Lock() @@ -146,55 +156,63 @@ func (arena *Arena) NextDisplayId() string { } } -// Adds the given display to the arena registry and triggers a notification. -func (arena *Arena) RegisterDisplay(display *Display) { +// Creates or gets the given display in the arena registry and triggers a notification. +func (arena *Arena) RegisterDisplay(displayConfig *DisplayConfiguration, ipAddress string) *Display { displayRegistryMutex.Lock() defer displayRegistryMutex.Unlock() - existingDisplay, ok := arena.Displays[display.Id] - if ok && display.Type == PlaceholderDisplay { + display, ok := arena.Displays[displayConfig.Id] + if ok && displayConfig.Type == PlaceholderDisplay { // Don't rewrite the registered configuration if the new one is a placeholder -- if it is reconnecting after a // restart, it should adopt the existing configuration. - arena.Displays[display.Id].ConnectionCount++ + arena.Displays[displayConfig.Id].ConnectionCount++ + arena.Displays[displayConfig.Id].IpAddress = ipAddress } else { - if ok { - display.ConnectionCount = existingDisplay.ConnectionCount + 1 - } else { - display.ConnectionCount = 1 + if !ok { + display = new(Display) + display.Notifier = websocket.NewNotifier("displayConfiguration", + display.generateDisplayConfigurationMessage) + arena.Displays[displayConfig.Id] = display } + display.DisplayConfiguration = *displayConfig + display.IpAddress = ipAddress + display.ConnectionCount += 1 display.lastConnectedTime = time.Now() - arena.Displays[display.Id] = display + display.Notifier.Notify() } arena.DisplayConfigurationNotifier.Notify() + + return display } // Updates the given display in the arena registry. Triggers a notification if the display configuration changed. -func (arena *Arena) UpdateDisplay(display *Display) error { +func (arena *Arena) UpdateDisplay(displayConfig DisplayConfiguration) error { displayRegistryMutex.Lock() defer displayRegistryMutex.Unlock() - existingDisplay, ok := arena.Displays[display.Id] + display, ok := arena.Displays[displayConfig.Id] if !ok { - return fmt.Errorf("Display %s doesn't exist.", display.Id) + return fmt.Errorf("Display %s doesn't exist.", displayConfig.Id) } - display.ConnectionCount = existingDisplay.ConnectionCount - if !reflect.DeepEqual(existingDisplay, display) { - arena.Displays[display.Id] = display + if !reflect.DeepEqual(displayConfig, display.DisplayConfiguration) { + display.DisplayConfiguration = displayConfig + display.Notifier.Notify() arena.DisplayConfigurationNotifier.Notify() } return nil } // Marks the given display as having disconnected in the arena registry and triggers a notification. -func (arena *Arena) MarkDisplayDisconnected(display *Display) { +func (arena *Arena) MarkDisplayDisconnected(displayId string) { displayRegistryMutex.Lock() defer displayRegistryMutex.Unlock() - if existingDisplay, ok := arena.Displays[display.Id]; ok { - if existingDisplay.Type == PlaceholderDisplay && existingDisplay.Nickname == "" && - len(existingDisplay.Configuration) == 0 { + if existingDisplay, ok := arena.Displays[displayId]; ok { + if existingDisplay.DisplayConfiguration.Type == PlaceholderDisplay && + existingDisplay.DisplayConfiguration.Nickname == "" && + len(existingDisplay.DisplayConfiguration.Configuration) == 0 { // If the display is an unconfigured placeholder, just remove it entirely to prevent clutter. - delete(arena.Displays, existingDisplay.Id) + delete(arena.Displays, existingDisplay.DisplayConfiguration.Id) } else { existingDisplay.ConnectionCount -= 1 } @@ -210,7 +228,7 @@ func (arena *Arena) purgeDisconnectedDisplays() { deleted := false for id, display := range arena.Displays { - if display.ConnectionCount == 0 && display.Nickname == "" && + if display.ConnectionCount == 0 && display.DisplayConfiguration.Nickname == "" && time.Now().Sub(display.lastConnectedTime).Minutes() >= displayPurgeTtlMin { delete(arena.Displays, id) deleted = true diff --git a/field/display_test.go b/field/display_test.go index 0048c01..14736cc 100644 --- a/field/display_test.go +++ b/field/display_test.go @@ -52,8 +52,8 @@ func TestDisplayFromUrl(t *testing.T) { } func TestDisplayToUrl(t *testing.T) { - display := &Display{Id: "254", Nickname: "Test Nickname", Type: PitDisplay, - Configuration: map[string]string{"f": "1", "z": "#fff", "a": "3", "c": "4"}} + display := &Display{DisplayConfiguration: DisplayConfiguration{Id: "254", Nickname: "Test Nickname", + Type: PitDisplay, Configuration: map[string]string{"f": "1", "z": "#fff", "a": "3", "c": "4"}}} assert.Equal(t, "/displays/pit?displayId=254&nickname=Test+Nickname&a=3&c=4&f=1&z=%23fff", display.ToUrl()) } @@ -62,51 +62,57 @@ func TestNextDisplayId(t *testing.T) { assert.Equal(t, "100", arena.NextDisplayId()) - display := &Display{Id: "100"} - arena.RegisterDisplay(display) + displayConfig := &DisplayConfiguration{Id: "100"} + arena.RegisterDisplay(displayConfig, "") assert.Equal(t, "101", arena.NextDisplayId()) } func TestDisplayRegisterUnregister(t *testing.T) { arena := setupTestArena(t) - display := &Display{Id: "254", Nickname: "Placeholder", Type: PlaceholderDisplay, Configuration: map[string]string{}} - arena.RegisterDisplay(display) + displayConfig := &DisplayConfiguration{Id: "254", Nickname: "Placeholder", Type: PlaceholderDisplay, + Configuration: map[string]string{}} + arena.RegisterDisplay(displayConfig, "1.2.3.4") if assert.Contains(t, arena.Displays, "254") { - assert.Equal(t, "Placeholder", arena.Displays["254"].Nickname) - assert.Equal(t, PlaceholderDisplay, arena.Displays["254"].Type) + assert.Equal(t, "Placeholder", arena.Displays["254"].DisplayConfiguration.Nickname) + assert.Equal(t, PlaceholderDisplay, arena.Displays["254"].DisplayConfiguration.Type) assert.Equal(t, 1, arena.Displays["254"].ConnectionCount) + assert.Equal(t, "1.2.3.4", arena.Displays["254"].IpAddress) } + notifier := arena.Displays["254"].Notifier // Register a second instance of the same display. - display2 := &Display{Id: "254", Nickname: "Pit", Type: PitDisplay, Configuration: map[string]string{}} - arena.RegisterDisplay(display2) + displayConfig2 := &DisplayConfiguration{Id: "254", Nickname: "Pit", Type: PitDisplay, + Configuration: map[string]string{}} + arena.RegisterDisplay(displayConfig2, "2.3.4.5") if assert.Contains(t, arena.Displays, "254") { - assert.Equal(t, "Pit", arena.Displays["254"].Nickname) - assert.Equal(t, PitDisplay, arena.Displays["254"].Type) + assert.Equal(t, "Pit", arena.Displays["254"].DisplayConfiguration.Nickname) + assert.Equal(t, PitDisplay, arena.Displays["254"].DisplayConfiguration.Type) assert.Equal(t, 2, arena.Displays["254"].ConnectionCount) + assert.Equal(t, "2.3.4.5", arena.Displays["254"].IpAddress) + assert.Same(t, notifier, arena.Displays["254"].Notifier) } // Register a second display. - display3 := &Display{Id: "148", Type: FieldMonitorDisplay, Configuration: map[string]string{}} - arena.RegisterDisplay(display3) + displayConfig3 := &DisplayConfiguration{Id: "148", Type: FieldMonitorDisplay, Configuration: map[string]string{}} + arena.RegisterDisplay(displayConfig3, "3.4.5.6") if assert.Contains(t, arena.Displays, "148") { assert.Equal(t, 1, arena.Displays["148"].ConnectionCount) } // Update the first display. - display4 := &Display{Id: "254", Nickname: "Alliance", Type: AllianceStationDisplay, + displayConfig4 := DisplayConfiguration{Id: "254", Nickname: "Alliance", Type: AllianceStationDisplay, Configuration: map[string]string{"station": "B2"}} - arena.UpdateDisplay(display4) + arena.UpdateDisplay(displayConfig4) if assert.Contains(t, arena.Displays, "254") { - assert.Equal(t, "Alliance", arena.Displays["254"].Nickname) - assert.Equal(t, AllianceStationDisplay, arena.Displays["254"].Type) + assert.Equal(t, "Alliance", arena.Displays["254"].DisplayConfiguration.Nickname) + assert.Equal(t, AllianceStationDisplay, arena.Displays["254"].DisplayConfiguration.Type) assert.Equal(t, 2, arena.Displays["254"].ConnectionCount) } // Disconnect both displays. - arena.MarkDisplayDisconnected(display) - arena.MarkDisplayDisconnected(display3) + arena.MarkDisplayDisconnected(displayConfig.Id) + arena.MarkDisplayDisconnected(displayConfig3.Id) if assert.Contains(t, arena.Displays, "148") { assert.Equal(t, 0, arena.Displays["148"].ConnectionCount) } @@ -118,8 +124,8 @@ func TestDisplayRegisterUnregister(t *testing.T) { func TestDisplayUpdateError(t *testing.T) { arena := setupTestArena(t) - display := &Display{Id: "254", Configuration: map[string]string{}} - err := arena.UpdateDisplay(display) + displayConfig := DisplayConfiguration{Id: "254", Configuration: map[string]string{}} + err := arena.UpdateDisplay(displayConfig) if assert.NotNil(t, err) { assert.Contains(t, err.Error(), "doesn't exist") } @@ -129,41 +135,41 @@ func TestDisplayPurge(t *testing.T) { arena := setupTestArena(t) // Unnamed placeholder gets immediately purged upon disconnection. - display := &Display{Id: "254", Type: PlaceholderDisplay, Configuration: map[string]string{}} - arena.RegisterDisplay(display) + displayConfig := &DisplayConfiguration{Id: "254", Type: PlaceholderDisplay, Configuration: map[string]string{}} + arena.RegisterDisplay(displayConfig, "1.2.3.4") assert.Contains(t, arena.Displays, "254") - arena.MarkDisplayDisconnected(display) + arena.MarkDisplayDisconnected(displayConfig.Id) assert.NotContains(t, arena.Displays, "254") // Named placeholder does not get immediately purged upon disconnection. - display.Nickname = "Bob" - arena.RegisterDisplay(display) + displayConfig.Nickname = "Bob" + arena.RegisterDisplay(displayConfig, "1.2.3.4") assert.Contains(t, arena.Displays, "254") - arena.MarkDisplayDisconnected(display) + arena.MarkDisplayDisconnected(displayConfig.Id) assert.Contains(t, arena.Displays, "254") - // Unnamed configured display does not get immediately purged upon disconnection. - display = &Display{Id: "1114", Type: FieldMonitorDisplay, Configuration: map[string]string{}} - arena.RegisterDisplay(display) + // Unnamed configured displayConfig does not get immediately purged upon disconnection. + displayConfig = &DisplayConfiguration{Id: "1114", Type: FieldMonitorDisplay, Configuration: map[string]string{}} + arena.RegisterDisplay(displayConfig, "1.2.3.4") assert.Contains(t, arena.Displays, "1114") - arena.MarkDisplayDisconnected(display) + arena.MarkDisplayDisconnected(displayConfig.Id) assert.Contains(t, arena.Displays, "1114") arena.purgeDisconnectedDisplays() assert.Contains(t, arena.Displays, "1114") - // Unnamed configured display gets purged by periodic task. - arena.RegisterDisplay(display) + // Unnamed configured displayConfig gets purged by periodic task. + arena.RegisterDisplay(displayConfig, "1.2.3.4") assert.Contains(t, arena.Displays, "1114") - arena.MarkDisplayDisconnected(display) + arena.MarkDisplayDisconnected(displayConfig.Id) arena.Displays["1114"].lastConnectedTime = time.Now().Add(-displayPurgeTtlMin * time.Minute) arena.purgeDisconnectedDisplays() assert.NotContains(t, arena.Displays, "1114") - // Named configured display does not get purged by periodic task. - display.Nickname = "Brunhilda" - arena.RegisterDisplay(display) + // Named configured displayConfig does not get purged by periodic task. + displayConfig.Nickname = "Brunhilda" + arena.RegisterDisplay(displayConfig, "1.2.3.4") assert.Contains(t, arena.Displays, "1114") - arena.MarkDisplayDisconnected(display) + arena.MarkDisplayDisconnected(displayConfig.Id) arena.Displays["1114"].lastConnectedTime = time.Now().Add(-displayPurgeTtlMin * time.Minute) arena.purgeDisconnectedDisplays() assert.Contains(t, arena.Displays, "1114") diff --git a/static/js/cheesy-websocket.js b/static/js/cheesy-websocket.js index 980326e..044d804 100644 --- a/static/js/cheesy-websocket.js +++ b/static/js/cheesy-websocket.js @@ -40,13 +40,11 @@ var CheesyWebsocket = function(path, events) { // Insert an event to allow reconfiguration if this is a display. if (!events.hasOwnProperty("displayConfiguration")) { events.displayConfiguration = function (event) { - if (displayId in event.data.DisplayUrls) { - var newUrl = event.data.DisplayUrls[displayId]; + var newUrl = event.data; - // Reload the display if the configuration has changed. - if (newUrl !== window.location.pathname + window.location.search) { - window.location = newUrl; - } + // Reload the display if the configuration has changed. + if (newUrl !== window.location.pathname + window.location.search) { + window.location = newUrl; } }; } diff --git a/static/js/setup_displays.js b/static/js/setup_displays.js index 0609a61..359dc2f 100644 --- a/static/js/setup_displays.js +++ b/static/js/setup_displays.js @@ -38,14 +38,14 @@ var reloadAllDisplays = function() { var handleDisplayConfiguration = function(data) { $("#displayContainer").empty(); - $.each(data.Displays, function(displayId, display) { + $.each(data, function(displayId, display) { var displayRow = displayTemplate(display); $("#displayContainer").append(displayRow); - $("#displayNickname" + displayId).val(display.Nickname); - $("#displayType" + displayId).val(display.Type); + $("#displayNickname" + displayId).val(display.DisplayConfiguration.Nickname); + $("#displayType" + displayId).val(display.DisplayConfiguration.Type); // Convert configuration map to query string format. - var configurationString = $.map(Object.entries(display.Configuration), function(entry) { + var configurationString = $.map(Object.entries(display.DisplayConfiguration.Configuration), function(entry) { return entry.join("="); }).join("&"); $("#displayConfiguration" + displayId).val(configurationString); diff --git a/templates/setup_displays.html b/templates/setup_displays.html index cb7e0a9..1004df8 100644 --- a/templates/setup_displays.html +++ b/templates/setup_displays.html @@ -31,30 +31,30 @@