nuttxpr commented on PR #14591: URL: https://github.com/apache/nuttx/pull/14591#issuecomment-2451583259
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) __Fill In The Commit Message:__ This PR contains a Commit with an Empty Commit Message. Please fill in the Commit Message with the PR Summary. The PR summary is missing key information required by the NuttX guidelines. While it explains the "why," it doesn't adequately cover the "what" and "how." The impact section is also insufficient. Here's a breakdown of what needs improvement and a suggested rewrite: **Problems:** * **Summary - What:** It mentions adding I2C bitbang support, but doesn't specify which files were modified or new files added. Be explicit. For example: "Modified `arch/risc-v/src/esp32c3/esp32c3_i2c.c` to add bitbang support..." * **Summary - How:** It lacks detail about the implementation. Did you use a specific bitbang library? What GPIO control mechanisms are employed? Briefly explain the core logic of the bitbang implementation. * **Impact - Insufficient Detail:** Just listing the chips isn't enough. You must address *each* point in the impact section with "YES" or "NO" and provide descriptions for any "YES" answers. This is crucial. * **Testing - Before Change Logs:** Where are the logs from *before* the change? These are essential to show the issue existed and that your change fixed it. Even if adding a new feature, show the absence of the functionality before your changes. * **Issue References:** Are there related NuttX issues? If so, include them. **Suggested Rewrite and Improvements:** ``` ## Summary This change adds I2C bitbang support to ESP32, ESP32-S2, ESP32-S3, ESP32-C3, ESP32-C6, and ESP32-H2 architectures. This is necessary to provide additional I2C interfaces for devices with limited hardware I2C peripherals. This allows for greater flexibility in device testing and supports use cases where multiple I2C devices are required. The following files were modified to implement this feature: * `arch/risc-v/src/esp32c3/esp32c3_i2c.c` * `arch/risc-v/src/esp32c6/esp32c6_i2c.c` * `arch/risc-v/src/esp32h2/esp32h2_i2c.c` * `arch/risc-v/src/esp32s2/esp32s2_i2c.c` * `arch/risc-v/src/esp32s3/esp32s3_i2c.c` * ... (list all modified files) The implementation uses a software-based bitbanging technique leveraging the GPIO subsystem. GPIO pins are configured as outputs for SCL and SDA, and their states are manipulated to emulate the I2C protocol. ...(Provide a concise summary of the core bitbang logic, e.g., how start/stop conditions and data transfer are handled). Related NuttX Issue: #[If applicable, put issue number here] ## Impact * Is new feature added? YES (I2C bitbang support) * Is existing feature changed? NO * Impact on user (will user need to adapt to change)? YES (Users will need to enable the `ESPRESSIF_I2C_BITBANG` Kconfig option and configure the GPIO pins for SCL and SDA.) * Impact on build (will build process change)? NO * Impact on hardware (will arch(s) / board(s) / driver(s) change)? YES (This change affects the I2C drivers for the ESP32, ESP32-S2, ESP32-S3, ESP32-C3, ESP32-C6, and ESP32-H2 architectures.) * Impact on documentation (is update required / provided)? YES (Documentation will be updated to explain how to enable and configure the I2C bitbang feature.) * Impact on security (any sort of implications)? NO * Impact on compatibility (backward/forward/interoperability)? NO * Anything else to consider? NO ## Testing Build Host(s): Linux (Ubuntu 20.04), x86_64, GCC 9.4.0 Target(s): esp32c3-generic, esp32c6-devkitc, esp32h2-devkit, esp32s3-devkit Testing logs before change: ``` nsh> i2c dev -b 3 // Show that I2C bus 3 is not available or functional. (Example output demonstrating the lack of bitbang functionality) nsh> bmp180 // Show that bmp180 fails because I2C bus is not available. (Example output demonstrating the failure) ``` Testing logs after change: ``` (Your provided logs demonstrating successful communication with the BMP180 sensor) ``` ``` By providing more specific details and addressing all the points in the template, you'll make it much easier for reviewers to understand and accept your PR. Remember to always include "before" logs and link to any related issues. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
