Add runtime game selection with web interface by Skeyelab · Pull Request #1 · Skeyelab/ESP32-Game

Skeyelab · 2025-12-16T18:21:42Z

Summary

Implements runtime game selection allowing users to switch between all 11 games via a web interface without recompiling. The selected game is persisted in EEPROM.

Changes

Game Manager System: New game_manager.h/cpp with game registry and EEPROM persistence
Game Wrappers: Added unique wrapper functions (game_XX_setup, game_XX_loop) to each game file
Web Interface:
- New endpoints: GET /game/current, POST /game/select, updated GET /games
- Enhanced dashboard with game selection UI (button grid + dropdown)
Build System: Removed games filter from platformio.ini so all games compile together
Networking: Default to AP mode (self-hosted server) instead of WiFi connection
Tests: Added basic test structure for game manager

Technical Details

All games now compile into a single firmware image
Game selection persists across power cycles via EEPROM
Web server provides real-time game status and LED visualization
All 11 games accessible via web interface

Testing

✅ Compilation successful with all games included
✅ Basic test structure created (needs EEPROM mocking for full testing)
✅ Web interface tested manually

Breaking Changes

Removed compile-time game selection (#define GAME_XX)
All games must now be compiled together

- Implement game manager system with EEPROM persistence - Add unique wrapper functions for each game (game_XX_setup/loop) - Update main.cpp to use game manager instead of compile-time selection - Add web server endpoints: GET /game/current, POST /game/select - Update web dashboard with game selection UI (buttons + dropdown) - Make all games compile together (removed games filter from platformio.ini) - Add basic test structure for game manager - Default to AP mode (self-hosted server) instead of WiFi connection

- Run unit tests on native environment - Build ESP32 firmware - Upload firmware as artifact - Cache PlatformIO dependencies for faster builds

…t action

Copilot

Pull request overview

This PR implements runtime game selection for an ESP32-based LED game system, allowing users to switch between all 11 games via a web interface without recompiling. The selected game persists in EEPROM across power cycles.

Key Changes:

Introduced a game manager system with EEPROM persistence and function pointer registry
Added web interface with REST API endpoints for game selection and status monitoring
Modified all game files to use static internal functions with public wrapper functions to enable compilation of all games together

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 17 comments.

Show a summary per file

File	Description
`src/games/game_manager.h/cpp`	Core game manager with registry, EEPROM persistence, and game switching logic
`src/games/game__.cpp`	All 11 game files modified to add static internal functions and public wrappers for game manager integration
`src/network/web_server.h/cpp`	Web server with HTML dashboard and REST API endpoints for game selection and status
`src/network/wifi_manager.h/cpp`	WiFi connection manager with AP mode fallback support
`src/network/mqtt_client.h/cpp`	MQTT client for remote status publishing (disabled by default)
`src/status/status_monitor.h/cpp`	Status tracking system that monitors game state, score, input, and LED state changes
`src/main.cpp`	Main loop refactored to use game manager, with optional networking integration
`src/config/wifi_config.h`	WiFi and AP mode configuration with credentials
`src/config/mqtt_config.h`	MQTT broker configuration settings
`test/test_game_manager/test_main.cpp`	Test structure with mocks for game manager (placeholder implementations only)
`platformio.ini`	Updated to include PubSubClient and ArduinoJson dependencies
`README.md`	Documentation added for networking features, web interface, and MQTT topics

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-16T18:42:38Z

+void test_game_count() {
+  // Should have 11 games (0-10)
+  const uint8_t expected_count = 11;
+  // In real implementation: TEST_ASSERT_EQUAL(expected_count, game_manager_get_game_count());
+  TEST_ASSERT_TRUE(expected_count == 11);
+}
+
+// Test get game info valid ID
+void test_get_game_info_valid() {
+  // Should return valid GameInfo for ID 0
+  // In real: const GameInfo* info = game_manager_get_game_info(0);
+  // TEST_ASSERT_NOT_NULL(info);
+  // TEST_ASSERT_EQUAL(0, info->id);
+  // TEST_ASSERT_EQUAL_STRING("Test", info->name);
+  TEST_ASSERT_TRUE(true); // Placeholder
+}
+
+// Test get game info invalid ID
+void test_get_game_info_invalid() {
+  // Should return nullptr for ID >= 11
+  // In real: TEST_ASSERT_NULL(game_manager_get_game_info(11));
+  // TEST_ASSERT_NULL(game_manager_get_game_info(255));
+  TEST_ASSERT_TRUE(true); // Placeholder
+}
+
+// Test set game valid ID
+void test_set_game_valid() {
+  // Should successfully set game to valid ID
+  // In real: TEST_ASSERT_TRUE(game_manager_set_game(1));
+  // TEST_ASSERT_EQUAL(1, game_manager_get_current_game());
+  TEST_ASSERT_TRUE(true); // Placeholder
+}
+
+// Test set game invalid ID
+void test_set_game_invalid() {
+  // Should return false for invalid ID
+  // In real: TEST_ASSERT_FALSE(game_manager_set_game(11));
+  // TEST_ASSERT_FALSE(game_manager_set_game(255));
+  TEST_ASSERT_TRUE(true); // Placeholder
+}
+
+// Test get current game name
+void test_get_current_game_name() {
+  // Should return correct name for current game
+  // In real: game_manager_set_game(0);
+  // TEST_ASSERT_EQUAL_STRING("Test", game_manager_get_current_game_name());
+  // game_manager_set_game(1);
+  // TEST_ASSERT_EQUAL_STRING("Pacman", game_manager_get_current_game_name());
+  TEST_ASSERT_TRUE(true); // Placeholder
+}
+
+// Test EEPROM persistence
+void test_eeprom_persistence() {
+  // After setting game, EEPROM should be written
+  // In real: game_manager_set_game(5);
+  // TEST_ASSERT_EQUAL(5, EEPROM_read(0));
+  TEST_ASSERT_TRUE(true); // Placeholder
+}
+
+// Test EEPROM validation
+void test_eeprom_validation() {
+  // Invalid EEPROM value should default to game 0
+  // In real: mock_eeprom[0] = 255; // Invalid
+  // game_manager_init();
+  // TEST_ASSERT_EQUAL(0, game_manager_get_current_game());
+  TEST_ASSERT_TRUE(true); // Placeholder
+}
+
+// Test game setup is called
+void test_game_setup_called() {
+  // When switching games, setup should be called
+  // In real: game_00_setup_called = false;
+  // game_manager_set_game(0);
+  // TEST_ASSERT_TRUE(game_00_setup_called);
+  TEST_ASSERT_TRUE(true); // Placeholder
+}
+
+// Test game loop is called
+void test_game_loop_called() {
+  // When calling game_manager_loop, the current game's loop should be called
+  // In real: game_00_loop_called = false;
+  // game_manager_set_game(0);
+  // game_manager_loop(33);
+  // TEST_ASSERT_TRUE(game_00_loop_called);
+  // TEST_ASSERT_EQUAL(33, last_loop_dt);
+  TEST_ASSERT_TRUE(true); // Placeholder
+}
+
+// Test game switching
+void test_game_switching() {
+  // Should be able to switch between games
+  // In real: game_manager_set_game(0);
+  // TEST_ASSERT_EQUAL(0, game_manager_get_current_game());
+  // game_manager_set_game(5);
+  // TEST_ASSERT_EQUAL(5, game_manager_get_current_game());
+  // game_manager_set_game(10);
+  // TEST_ASSERT_EQUAL(10, game_manager_get_current_game());
+  TEST_ASSERT_TRUE(true); // Placeholder
+}


All test cases are placeholder implementations that only execute TEST_ASSERT_TRUE(true), providing no actual test coverage for the game manager functionality. These tests should be fully implemented to verify game manager initialization, game selection, EEPROM persistence, and wrapper function calls. The mock infrastructure is in place (lines 1-54) but not being utilized.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2025-12-16T18:42:38Z

+    return;
+  }
+
+  server = new WebServer(80);


Memory leak: WebServer object is dynamically allocated but never deleted. If web_server_init() is called multiple times (although currently protected), or during shutdown, this memory will not be freed. Consider using a static WebServer object instead of a pointer, or implement proper cleanup in a web_server_deinit() function.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2025-12-16T18:42:38Z

+  while (WiFi.status() != WL_CONNECTED) {
+    if (millis() - connectStartTime > timeout_ms) {
+      wifiState = WIFI_STATE_DISCONNECTED;
+      return false;
+    }
+    delay(100);
+  }


Blocking delay in wifi_manager_connect() can cause the entire system to hang for extended periods. This is especially problematic since touch input and game loop execution will be blocked during WiFi connection attempts. Consider implementing a non-blocking connection approach or documenting that this should only be called during setup.

Copilot · 2025-12-16T18:42:38Z

+
+// AP mode fallback (if WiFi connection fails)
+#define AP_SSID "ESP32-Game"
+#define AP_PASSWORD ""


Security vulnerability: AP mode has no password (empty string), allowing anyone to connect to the ESP32 access point. This exposes the web interface and game control endpoints to unauthorized users. Set a default password or document that users must configure one before deployment.

Suggested change

#define AP_PASSWORD ""

// WARNING: Change the AP password before deploying to production!

#define AP_PASSWORD "esp32game1234"

Copilot · 2025-12-16T18:42:39Z

+void game_00_loop(uint32_t dt) {
+  game_loop(dt);
+}
+


Unnecessary blank lines at the end of the file. While this doesn't affect functionality, it's inconsistent with the single blank line ending used in other game files.

Suggested change

Copilot · 2025-12-16T18:42:41Z

+    // Add LED array - ensure all 8 LEDs are included
+    JsonArray ledsArray = doc.createNestedArray("leds");
+    for (int i = 0; i < 8; i++) {
+      JsonObject led = ledsArray.createNestedObject();
+      led["r"] = (int)status.leds[i].r;
+      led["g"] = (int)status.leds[i].g;
+      led["b"] = (int)status.leds[i].b;
+    }


Inconsistent indentation: This line uses extra indentation (appears to be 4 additional spaces) compared to surrounding lines. This should match the indentation of the surrounding code block for consistency.

Copilot · 2025-12-16T18:42:41Z

 }


+


Inconsistent blank lines: This file has only 1 blank line before the wrapper functions (line 249), while other game files like game_04_flappy.cpp have 5-6 blank lines in the same location. Consider standardizing the spacing between the main game logic and wrapper functions across all game files.

Suggested change

Copilot · 2025-12-16T18:42:41Z

+    // Call setup for the new game
+    if (GAMES[gameId].setup) {
+      GAMES[gameId].setup();
+    }


API design issue: game_manager_set_game() automatically calls the setup function for the new game, but this behavior may be unexpected. If called during an active game loop, this could cause state corruption or unexpected resets. Consider separating game selection from game initialization, or document this behavior clearly in the header file and add a safety check to prevent switching during active gameplay.

Suggested change

// Call setup for the new game

if (GAMES[gameId].setup) {

GAMES[gameId].setup();

}

// Note: Setup for the new game is NOT called automatically.

// Call game_manager_setup() after switching games to initialize the new game.

Copilot · 2025-12-16T18:42:42Z

+  // Publish full status as JSON (increased size for LED array)
+  StaticJsonDocument<768> doc;
+  doc["gameName"] = status.gameName;
+  doc["score"] = status.score;
+  doc["state"] = status.state;
+  doc["leftPressed"] = status.leftPressed;
+  doc["rightPressed"] = status.rightPressed;
+  doc["actionPressed"] = status.actionPressed;
+  doc["altPressed"] = status.altPressed;
+  doc["timestamp"] = status.timestamp;
+
+    // Add LED array - ensure all 8 LEDs are included
+    JsonArray ledsArray = doc.createNestedArray("leds");
+    for (int i = 0; i < 8; i++) {
+      JsonObject led = ledsArray.createNestedObject();
+      led["r"] = (int)status.leds[i].r;
+      led["g"] = (int)status.leds[i].g;
+      led["b"] = (int)status.leds[i].b;
+    }


Code duplication: The JSON serialization logic for game status (lines 108-126) duplicates the same logic from handleStatus() in web_server.cpp (lines 278-295). This creates maintenance issues if the status format needs to change. Consider extracting this into a shared helper function like serialize_game_status() to avoid duplication.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2025-12-16T18:42:42Z

+
+### Enabling Networking
+
+1. Uncomment `#define ENABLE_NETWORKING` in `src/main.cpp`


Missing backticks around code directive. The define should be formatted with backticks for consistency with other code snippets in the documentation.

@copilot open a new pull request to apply changes based on this feedback

Skeyelab requested a review from Copilot December 16, 2025 18:22

Copilot started reviewing on behalf of Skeyelab December 16, 2025 18:23 View session

Skeyelab added 2 commits December 16, 2025 13:23

Add GitHub Actions CI workflow

306005d

- Run unit tests on native environment - Build ESP32 firmware - Upload firmware as artifact - Cache PlatformIO dependencies for faster builds

Fix GitHub Actions: Install PlatformIO via pip instead of non-existen…

242bd1a

…t action

Skeyelab merged commit fd58d88 into main Dec 16, 2025
4 checks passed

Copilot AI reviewed Dec 16, 2025

View reviewed changes

	#define AP_PASSWORD ""
	// WARNING: Change the AP password before deploying to production!
	#define AP_PASSWORD "esp32game1234"


		### Enabling Networking

		1. Uncomment `#define ENABLE_NETWORKING` in `src/main.cpp`

Conversation

Skeyelab commented Dec 16, 2025

Summary

Changes

Technical Details

Testing

Breaking Changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Skeyelab Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Skeyelab Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Skeyelab Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Skeyelab Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants