Skip to content

RP2040 quality pass: 5 correctness/robustness fixes#10

Open
semichcsc-byte wants to merge 1 commit into
Concept-Bytes:mainfrom
semichcsc-byte:fix/rp2040-quality-pass
Open

RP2040 quality pass: 5 correctness/robustness fixes#10
semichcsc-byte wants to merge 1 commit into
Concept-Bytes:mainfrom
semichcsc-byte:fix/rp2040-quality-pass

Conversation

@semichcsc-byte
Copy link
Copy Markdown

Summary

Five small fixes for the RP2040 build path. All are user-visible, low-risk, and validated on an Arduino Nano RP2040 Connect with WiFiNINA firmware 3.0.1. 6 files changed, +67/-12 lines.

This is a follow-up to #9 (the AP→STA WiFi fix). Same hardware, same firmware path. Targeting the user with the official Concept-Bytes PCB + Nano RP2040 setup.


1. stockfish_settings.hmedium() depth bug

 static StockfishSettings medium() {
     StockfishSettings s;
-    s.depth = 6;
+    s.depth = 10;
     s.timeoutMs = 25000;
     return s;
 }

The serial banner says 'Medium (Depth 10)' but the struct sent depth=6 — making Easy and Medium identical. Almost certainly a copy-paste leftover.

2. chess_bot.cpp::parseStockfishResponse — split HTTP headers from body

The previous parser scanned the full HTTP response with indexOf("{"). Cloudflare's response includes JSON-shaped headers before the body:

HTTP/1.1 200 OK
Date: ...
Nel: {"report_to":"cf-nel",...}     ← first {  ❌ WRONG
Report-To: {"group":"cf-nel",...}
...

{"success":true,"bestmove":"bestmove e2e4 ..."}     ← real body

Effect: Extracted JSON: {"report_to":"cf-nel",...} then API request was not successful on a successful response. Today it accidentally still works because Nel doesn't contain the substring \"success\":true, but the next time Cloudflare adjusts its headers we break.

Fix: locate the body after the first blank line (\\r\\n\\r\\n) and parse from there. Falls back gracefully if only \\n\\n separators are present.

3. chess_bot.cpp::makeBotMove — validate API response locally

executeBotMove previously applied the API's response directly to the board with no sanity checking. Adds a guard that:

  • rejects moves whose source square is empty (sign of a stale/garbled response),
  • rejects moves the local ChessEngine::isValidMove won't accept,
  • on any rejection or upstream failure path, returns control to the player (isWhiteTurn = true) instead of leaving the user with frozen LEDs and botThinking stuck false.

Cheap insurance against malformed JSON, partial reads, and side-of-turn confusion.

4. OpenChess.ino MODE_GAME_3 — kill the placeholder loop

If the user puts a piece on the 'Coming Soon' selector (4,3), the loop spammed:

This game mode will be available in a future update!
Returning to game selection in 3 seconds...

every 3 seconds forever, because handleGameSelection re-triggered the same case as long as the sensor was active.

Fix: wait until the sensor at (4,3) clears before re-arming the selector menu. No more spam, and selecting another mode immediately works.

5. chess_engine.hMAX_MOVES_PER_PIECE constant

Was inconsistent across the codebase:

  • chess_moves.cpp: int moves[28][2]
  • chess_bot.cpp: int moves[27][2] (twice — buffer overflow risk for a queen with 27 candidate moves)
  • chess_engine.cpp::isValidMove: int moves[28][2]

Centralise as #define MAX_MOVES_PER_PIECE 28 in chess_engine.h and use it everywhere. Eliminates the off-by-one risk and keeps callers in sync.


Verification

Recompiled with arduino-cli compile --fqbn arduino:mbed_nano:nanorp2040connect:

Sketch uses 144717 bytes (0%) of program storage space.
Global variables use 44440 bytes (16%) of dynamic memory.

Reflashed onto the same Nano RP2040 Connect used in #9. Boot output unchanged. Bot mode still connects via the patch from #9, parses Stockfish replies and plays.

Trade-offs

  • +67 / -12 lines, 6 files, no API changes, no behaviour changes for working code paths
  • The MODE_GAME_3 wait blocks the loop while a piece sits on the placeholder square — that is intentional and matches user expectation ("I selected the wrong square, let me move my piece").
  • Sketch size +337 bytes (negligible on RP2040's 16MB)

1) stockfish_settings.h: medium() depth 6 -> 10
   Bug: serial said 'Medium (Depth 10)' but the struct sent depth=6,
   making Easy and Medium identical. Aligns with the documented intent.

2) chess_bot.cpp::parseStockfishResponse: split HTTP headers from body
   before scanning for JSON. Cloudflare's response includes JSON-shaped
   headers (Nel:, Report-To:) that come before the real body, so the
   previous indexOf("{") grabbed the wrong object. Symptom: 'API request
   was not successful' on a successful response. Fix: locate the body
   after the first blank line (\\r\\n\\r\\n) and parse from there.

3) chess_bot.cpp::makeBotMove: validate the API's move locally before
   mutating board state. Protects against malformed JSON, partial reads,
   side-of-turn mismatches and any future API quirk. On rejection the
   board returns to the player's turn instead of corrupting state.
   Also reset isWhiteTurn on parse/response failure paths so the user
   isn't stuck.

4) OpenChess.ino MODE_GAME_3: replace blind 3s loop with a wait-until-
   piece-lifted gate. Old behaviour spammed the serial at ~3 lines/sec
   forever while a piece sat on the placeholder square (4,3) since
   handleGameSelection re-triggered the same branch every cycle. Now we
   wait for the sensor to clear before re-arming the menu.

5) chess_engine.h: introduce MAX_MOVES_PER_PIECE constant (=28) and use
   it everywhere. Was inconsistent: chess_moves used moves[28], chess_bot
   used moves[27]. Eliminates the off-by-one buffer-overflow risk for
   queens reaching 27 candidates.

All five are RP2040-friendly and apply equally to other WiFiNINA boards
(Nano 33 IoT, MKR WiFi 1010). Verified by recompiling and reflashing
the official OpenChess.ino on an Arduino Nano RP2040 Connect; bot mode
still connects, parses Stockfish replies and plays correctly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant