-
Notifications
You must be signed in to change notification settings - Fork 171
Develop #178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
fixed Sewrial issue esp32s2
Retry 3 times camera init
Co-authored-by: Nick Eales <[email protected]>
* add support for M5PoECAM-W (U121-B) * update documentation for newly added M5PoECAM-W * update board values for m5poecam-w
…king 133-esp32-cam-led-flash-not-working
Add the m5stack-timer-cam definitions following information from the board documentation [1] and the board entry on platform.io [2]. Tweak the upload speed to use a known working value. Mention the X version in the readme as well since it uses the same board and is just a chassis change. [1] https://docs.m5stack.com/en/unit/timercam [2] https://github.com/platformio/platform-espressif32/blob/ec69109ed6e2bb19464266357436c028a60560c3/boards/m5stack-timer-cam.json Signed-off-by: Randolph Sapp <[email protected]>
platformio.ini: add m5stack-timer-cam
Added S3 wroom n16r8 board
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for new ESP32-CAM board variants and introduces flash LED control functionality. The changes include support for the M5Stack M5PoECAM-W, M5Stack Timer CAM, and ESP32 S3 WROOM n16r8 boards, along with HTTP endpoint for controlling flash LEDs on compatible boards. Additionally, the platform version is pinned to [email protected] for consistency, and indentation in camera configuration is standardized.
Key changes:
- Added flash LED control via HTTP endpoint
/flashfor boards with flash LED support - Moved Serial initialization earlier in setup to ensure debug output is available from the start
- Pinned platform version to [email protected] for reproducible builds
Reviewed Changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main.cpp | Added flash LED control function and HTTP handler; moved Serial.begin() earlier; standardized camera_config indentation |
| platformio.ini | Pinned platform version to [email protected]; added new board environments for m5stack-timer-cam and esp32cam_s3_wroom_n16r8 |
| boards/m5stack-timer-cam.json | New board definition for M5Stack Timer CAM with ESP32 and PSRAM support |
| boards/esp32cam_s3_wroom_n16r8.json | New board definition for ESP32-S3 WROOM with 16MB flash and 8MB PSRAM |
| boards/esp32cam_m5stack_m5poecam_w.json | New board definition for M5Stack M5PoECAM-W with ESP32 and PoE support |
| boards/esp32cam_ai_thinker.json | Added FLASH_LED_GPIO=4 definition for AI Thinker board |
| boards/esp32cam_m5stack_unitcams3.json | Updated camera pin assignments to correct GPIO mappings |
| README.md | Added new boards to supported list; updated PSRAM table; added August 2024 change history entry |
| dotnet_riscv | Removed subproject commit reference |
| assets/boards/*.webp | Added product images for M5Stack M5PoECAM-W |
Comments suppressed due to low confidence (1)
README.md:50
- The newly added "ESP32 S3 WROOM n16r8" board (defined in boards/esp32cam_s3_wroom_n16r8.json and platformio.ini) is missing from the list of supported ESP32-CAM modules. It should be added to maintain consistency with the other supported boards.
This software supports the following ESP32-CAM (and alike) modules:
- AI THINKER
- EspressIf ESP-EYE
- EspressIf ESP32S2-CAM
- EspressIf ESP32S3-CAM-LCD
- EspressIf ESP32S3-EYE
- Freenove WROVER KIT
- M5STACK ESP32CAM
- M5STACK_PSRAM
- M5STACK_UNITCAM
- M5STACK_UNITCAMS3
- M5STACK_V2_PSRAM
- M5STACK_PSRAM
- M5STACK_WIDE
- M5STACK M5PoECAM-W
- M5STACK Timer CAM (Original and X)
- M5STACK
- Seeed Studio XIAO ESP32S3 SENSE
- TTGO T-CAMERA
- TTGO T-JOURNAL
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .grab_mode = CAMERA_GRAB_LATEST, // When buffers should be filled | ||
| #if CONFIG_CAMERA_CONVERTER_ENABLED | ||
| conv_mode = CONV_DISABLE, // RGB<->YUV Conversion mode | ||
| conv_mode = CONV_DISABLE, // RGB<->YUV Conversion mode |
Copilot
AI
Nov 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The conv_mode field is missing a dot prefix (.) for designated initializer syntax. This should be .conv_mode = CONV_DISABLE to match the C99 designated initializer pattern used throughout the rest of the structure.
| conv_mode = CONV_DISABLE, // RGB<->YUV Conversion mode | |
| .conv_mode = CONV_DISABLE, // RGB<->YUV Conversion mode |
| ], | ||
| "name": "ESP32-CAM M5STACK M5PoECAM-W", | ||
| "upload": { | ||
| "flash_size": "16MB", |
Copilot
AI
Nov 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The flash size is inconsistent with the partition configuration. The board defines "partitions": "default_8MB.csv" (line 5) but specifies "flash_size": "16MB" (line 60). These should match - either both should be 8MB or both 16MB based on the actual hardware specifications.
| "flash_size": "16MB", | |
| "flash_size": "8MB", |
| | M5STACK_ESP32CAM | No | | ||
| | M5STACK_UNITCAM | No | | ||
| | M5STACK_UNITCAMS3 | 8Mb | | ||
| | M5STACK_M5PoECAM-W | 8MB | |
Copilot
AI
Nov 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The table formatting uses inconsistent units for PSRAM sizes. Most entries use "8Mb" (megabits) but this entry uses "8MB" (megabytes). For consistency with the rest of the table, this should be "8Mb".
| | M5STACK_M5PoECAM-W | 8MB | | |
| | M5STACK_M5PoECAM-W | 8Mb | |
| "url": "https://docs.espressif.com/projects/esp-idf/en/latest/esp32s3/hw-reference/esp32s3/user-guide-devkitc-1.html", | ||
| "vendor": "Espressif" |
Copilot
AI
Nov 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "url" and "vendor" fields have inconsistent indentation. They should be aligned at the same level as "upload" (2 spaces), but instead they have 4 spaces. This breaks the JSON structure consistency.
| "url": "https://docs.espressif.com/projects/esp-idf/en/latest/esp32s3/hw-reference/esp32s3/user-guide-devkitc-1.html", | |
| "vendor": "Espressif" | |
| "url": "https://docs.espressif.com/projects/esp-idf/en/latest/esp32s3/hw-reference/esp32s3/user-guide-devkitc-1.html", | |
| "vendor": "Espressif" |
| ## Change history | ||
|
|
||
| - August 2024 | ||
| - Added support for M5Stack M5PoECAM-W |
Copilot
AI
Nov 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change history entry only mentions "M5Stack M5PoECAM-W" but the PR also adds support for "M5Stack Timer CAM" and "ESP32 S3 WROOM n16r8" boards. These should be included in the change history for completeness.
| - Added support for M5Stack M5PoECAM-W | |
| - Added support for M5Stack M5PoECAM-W, M5Stack Timer CAM, and ESP32 S3 WROOM n16r8 |
| log_v("handle_flash"); | ||
| // If no value present, use off, otherwise convert v to integer. Depends on analog resolution for max value | ||
| auto v = web_server.hasArg("v") ? web_server.arg("v").toInt() : 0; | ||
| // If conversion fails, v = 0 |
Copilot
AI
Nov 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The handle_flash function lacks input validation for the v parameter. The toInt() method can return negative values or values exceeding the valid range (0-255 for 8-bit resolution). Consider adding validation to clamp the value: auto v = constrain(web_server.hasArg("v") ? web_server.arg("v").toInt() : 0, 0, 255);
| // If conversion fails, v = 0 | |
| v = constrain(v, 0, 255); // Clamp value to valid range for 8-bit PWM |
No description provided.