latest version - #19
Conversation
…lify coin submission, and configure static IP for M5Paper.
rotate check icon to user side
There was a problem hiding this comment.
Pull request overview
This PR implements a Web3 Showcase system with multiple interconnected IoT stations using M5Stack devices. The system includes user identity management, BLE-based authentication, coin earning/spending functionality, and inter-device communication via WiFi and ESP-NOW protocols.
Key changes:
- Implementation of 4 stations with different roles (identity/AP, authentication, earning coins, spending coins)
- Global configuration and device firmware for Core2 and StickC-Plus devices
- Network communication infrastructure using SoftAP, HTTP endpoints, and ESP-NOW
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 21 comments.
Show a summary per file
| File | Description |
|---|---|
| Station5/README.md | Documentation for system reset functionality |
| Station4/paper/paper.ino | M5Paper firmware for order/spend system with touch UI |
| Station4/paper/config.h | Configuration for Station 4 network and endpoints |
| Station4/matrix/matrix.ino | Atom Matrix firmware for order confirmation with ESP-NOW |
| Station4/matrix/config.h | Configuration for Station 4 Matrix device |
| Station4/echo/echo.ino | Atom Echo firmware for audio feedback |
| Station3/paper/paper.ino | M5Paper firmware for coin earning system |
| Station3/paper/config.h | Configuration for Station 3 network and endpoints |
| Station2/matrix/matrix.ino | Atom Matrix firmware for BLE scanning and authentication |
| Station2/echo/echo.ino | Atom Echo firmware for authentication audio feedback |
| Global/stick/m5stick-c-plus.cpp | Wearable device firmware with BLE broadcast and coin wallet |
| Global/core2/core2.cpp | Access Point and captive portal for user registration |
| Global/config/config.h | Global network configuration and endpoint definitions |
| .gitignore | Added macOS .DS_Store exclusion |
Critical Issues Found: There are several critical configuration mismatches that will prevent the system from functioning correctly, including inconsistent WiFi SSIDs, password configurations, IP addresses, and HTTP endpoint names across different components. These must be resolved before deployment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| stickCServer.on("/set_username", HTTP_POST, handleUsername); | ||
| stickCServer.on("/set_username", HTTP_GET, handleUsername); | ||
| stickCServer.on("/add_coin", HTTP_POST, handleCoin); |
There was a problem hiding this comment.
The endpoint used here is "/add_coin" but in all config.h files, the endpoint for earning coins is defined as ENDPOINT_EARN_COIN with value "/earn_coin". This inconsistency between the endpoint definition and usage will cause HTTP requests to fail. The code should use the endpoint defined in the config files or update the endpoint name to match.
| void sendUsernameToStickC(String username) { | ||
| if (WiFi.getMode() == WIFI_AP) { | ||
| HTTPClient http; | ||
| String url = "http://" + String(STICKC_IP) + ":" + String(STICKC_PORT) + "/set_username"; |
There was a problem hiding this comment.
The endpoint "/set_username" is used here but does not match any endpoint definition in the config files. According to the config files, the user endpoint is defined as ENDPOINT_SET_USER with value "/set_user". This mismatch will cause communication failures between Core2 and StickC.
| String url = "http://" + String(STICKC_IP) + ":" + String(STICKC_PORT) + "/set_username"; | |
| String url = "http://" + String(STICKC_IP) + ":" + String(STICKC_PORT) + "/set_user"; |
| #include <HTTPClient.h> | ||
|
|
||
| // 1. กำหนดค่า SoftAP | ||
| const char *ssid = "Web3Showcase_AP"; |
There was a problem hiding this comment.
The SSID is defined as "Web3Showcase_AP" (without underscore after "Web3") in this file, but in the config.h files for stations it is defined as "Web3_Showcase_AP" (with underscore). This inconsistency in SSID naming will prevent devices from connecting to the correct access point. All files should use the same SSID value.
| const char *ssid = "Web3Showcase_AP"; | |
| const char *ssid = "Web3_Showcase_AP"; |
|
|
||
| // --- SoftAP Configuration (Hosted by Station 1) --- | ||
| const char* AP_SSID = "Web3_Showcase_AP"; | ||
| const char* AP_PASSWORD = NULL; // Open network for easy Captive Portal |
There was a problem hiding this comment.
The comment states "Open network for easy Captive Portal" but the AP_PASSWORD is actually set to NULL, not a comment. The comment is misleading as it describes what should happen when using an open network, but doesn't clearly state that this IS an open network configuration. Consider clarifying to "NULL = Open network for easy Captive Portal" to make it explicit.
| const char* AP_PASSWORD = NULL; // Open network for easy Captive Portal | |
| const char* AP_PASSWORD = NULL; // NULL = Open network for easy Captive Portal |
| status_canvas.pushCanvas(0, 700, UPDATE_MODE_DU4); | ||
| } | ||
|
|
||
| void sumbitStatus(String msg1, String msg2) { |
There was a problem hiding this comment.
The function name "sumbitStatus" contains a typo. It should be "submitStatus" (with two 'b's). This typo makes the function name inconsistent with the verb "submit" and could cause confusion.
| void sumbitStatus(String msg1, String msg2) { | |
| void submitStatus(String msg1, String msg2) { |
There was a problem hiding this comment.
line 218
sumbitStatus(type, coin);
change name to submitStatus too
| #include <esp_gap_ble_api.h> // เพิ่ม Library นี้สำหรับเปลี่ยนชื่อแบบ Fast Mode | ||
|
|
||
| // --- CONFIGURATION --- | ||
| const char *SSID_AP = "Web3Showcase_AP"; |
There was a problem hiding this comment.
The SSID is defined as "Web3Showcase_AP" (without underscore after "Web3") in this file, but in the config.h files for stations it is defined as "Web3_Showcase_AP" (with underscore). This inconsistency in SSID naming will prevent devices from connecting to the correct access point. All files should use the same SSID value.
| const char *SSID_AP = "Web3Showcase_AP"; | |
| const char *SSID_AP = "Web3_Showcase_AP"; |
| // 1. กำหนดค่า SoftAP | ||
| const char *ssid = "Web3Showcase_AP"; | ||
| const char *password = "12345678"; | ||
| const char* STICKC_IP = "192.168.4.2"; |
There was a problem hiding this comment.
The STICKC_IP is hardcoded as "192.168.4.2" which conflicts with the IP address mapping defined in all config.h files where IP_STICKC is defined as 192.168.4.10. This mismatch will cause the Core2 to send requests to the wrong IP address, preventing communication with the StickC device.
| const char* STICKC_IP = "192.168.4.2"; | |
| const char* STICKC_IP = "192.168.4.10"; |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated 17 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (selectedChoice != 1 && x >= 130 && x <= 229) { | ||
| if (selectedChoice > 0) { | ||
| defaultSelectButton(selectedChoice); | ||
| } | ||
| selectedChoice = 1; | ||
| selectButton(selectedChoice); | ||
| canvas.pushCanvas(0, 0, UPDATE_MODE_DU4); | ||
| } | ||
| else if (selectedChoice != 2 && x >= 230 && x <= 329) { | ||
| if (selectedChoice > 0) { | ||
| defaultSelectButton(selectedChoice); | ||
| } | ||
| selectedChoice = 2; | ||
| selectButton(selectedChoice); | ||
| canvas.pushCanvas(0, 0, UPDATE_MODE_DU4); | ||
| } | ||
| else if (selectedChoice != 3 && x >= 330 && x <= 410) { | ||
| if (selectedChoice > 0) { | ||
| defaultSelectButton(selectedChoice); | ||
| } | ||
| selectedChoice = 3; | ||
| selectButton(selectedChoice); | ||
| canvas.pushCanvas(0, 0, UPDATE_MODE_DU4); | ||
| } | ||
| else if (selectedChoice != 4 && x >= 430 && x <= 510) { | ||
| if (selectedChoice > 0) { | ||
| defaultSelectButton(selectedChoice); | ||
| } | ||
| selectedChoice = 4; | ||
| selectButton(selectedChoice); | ||
| canvas.pushCanvas(0, 0, UPDATE_MODE_DU4); | ||
| } |
There was a problem hiding this comment.
Potential maintainability issue. The magic numbers 130, 229, 230, 329, 330, 410, 430, 510 represent touch coordinates but are hardcoded without explanation or named constants. Consider defining these as named constants with descriptive names to improve code readability and maintainability.
| if (selectedChoice == 0) { | ||
| msg = "48"; | ||
| } else if (selectedChoice == 1) { | ||
| msg = "49"; | ||
| } else if (selectedChoice == 2) { | ||
| msg = "50"; | ||
| } else if (selectedChoice == 3) { | ||
| msg = "51"; | ||
| } else if (selectedChoice == 4) { | ||
| msg = "52"; | ||
| } |
There was a problem hiding this comment.
Unreachable code detected. The condition at line 137 already checks if selectedChoice == 0 and returns early. Lines 142-152 contain a nested if-else chain that starts with checking if selectedChoice == 0, which will never be true at this point in execution. The entire if-else block will always take the first branch (selectedChoice == 0) which is unreachable, making the subsequent else-if branches also unreachable.
|
|
||
| // --- SoftAP Configuration (Hosted by Station 1) --- | ||
| const char* AP_SSID = "Web3_Showcase_AP"; | ||
| const char* AP_PASSWORD = "12345678"; // Open network for easy Captive Portal |
There was a problem hiding this comment.
Inconsistent comment on line 8. The comment uses Thai characters and abbreviations without clear English documentation. Comments should be in a consistent language (preferably English for international collaboration) and provide clear context about what the configuration represents.
| #include <esp_wifi.h> | ||
| #include "config.h" | ||
|
|
||
| //ใส่ MAC Address |
There was a problem hiding this comment.
Inconsistent comment formatting. Line 8 contains Thai text in the comment, which is inconsistent with the English comments used elsewhere in the file and across the codebase. For better maintainability and collaboration, comments should use consistent language and formatting.
| //ใส่ MAC Address | |
| // Set MAC Address |
| SoundState currentSoundState = IDLE; | ||
| unsigned long stateChangeTime = 0; | ||
|
|
||
| //ฟังก์ชันทำงานเมื่อได้รับข้อมูลกลับมาจากPaper |
There was a problem hiding this comment.
Inconsistent comment formatting. Lines 28, 38, and other locations use Thai language comments mixed with English, which reduces code maintainability and accessibility for international developers. Comments should use a consistent language throughout the codebase.
| //ฟังก์ชันทำงานเมื่อได้รับข้อมูลกลับมาจากPaper | |
| // Function called when data is received from Paper |
| status_canvas.pushCanvas(0, 700, UPDATE_MODE_DU4); | ||
| } | ||
|
|
||
| void sumbitStatus(String msg1, String msg2) { |
There was a problem hiding this comment.
Spelling error in function name. The function is named "sumbitStatus" but should be "submitStatus". This typo appears in both the function definition and its usage on line 218.
| #include <HTTPClient.h> | ||
| #include "config.h" | ||
|
|
||
| // สร้าง Canvas 2 ใบ (ใบใหญ่=เมนู, ใบเล็ก=Status) |
There was a problem hiding this comment.
Inconsistent comment formatting. Lines 7-9, 109, 138, 171 and others contain Thai language comments. For better code maintainability and accessibility, comments should use consistent language (preferably English) throughout the codebase.
| // สร้าง Canvas 2 ใบ (ใบใหญ่=เมนู, ใบเล็ก=Status) | |
| // Create 2 canvases (main menu = large, status = small) |
| // --- GLOBALS --- | ||
| AsyncWebServer stickCServer(LOCAL_PORT); | ||
| String currentUsername = "Not Registered"; | ||
| String authenStatus = "X"; |
There was a problem hiding this comment.
Naming inconsistency. The variable is named "authenStatus" but should be "authStatus" or "authenticationStatus" for clarity. The abbreviation "authen" is non-standard; the standard abbreviation is "auth".
| String authenStatus = "X"; | |
| String authStatus = "X"; |
| #include <BLEDevice.h> | ||
| #include <BLEUtils.h> | ||
| #include <BLEServer.h> | ||
| #include <esp_gap_ble_api.h> // เพิ่ม Library นี้สำหรับเปลี่ยนชื่อแบบ Fast Mode |
There was a problem hiding this comment.
Inconsistent comment formatting. Lines 8, 31, 37, 48, 52-54, 62, 67 and others contain Thai language comments. For better code maintainability and international collaboration, comments should use consistent language (preferably English) throughout the codebase.
| bool sendReceiveCoin(String coin_value) { | ||
| HTTPClient http; | ||
| String url = "http://" + IP_STICKC.toString() + ENDPOINT_EARN_COIN; | ||
| Serial.println("Sending " + coin_value + " to " + url); | ||
| http.begin(url); | ||
| http.addHeader("Content-Type", "application/x-www-form-urlencoded"); | ||
|
|
||
| String postData = "amount=" + coin_value; | ||
|
|
||
| int code = http.POST(postData); |
There was a problem hiding this comment.
The sendReceiveCoin function sends CCoin rewards to IP_STICKC over plain HTTP and includes only an amount field, with no authentication or integrity token. On the easily accessible AP network, any nearby client can replicate this unauthenticated request to the coin endpoint and mint arbitrary CCoin or tamper with users’ balances. Introduce a proper authentication mechanism for this API (e.g., a shared secret or signed token per device/user) and avoid relying solely on network location, ideally also protecting the request in transit to prevent tampering.
No description provided.