nuttxpr commented on PR #15795: URL: https://github.com/apache/nuttx/pull/15795#issuecomment-2646282153
[**\[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. This PR, as it stands, **does not meet the NuttX requirements**. While the idea is promising and addresses a real need, it's currently too incomplete to be considered a proper pull request. Here's why: * **Summary:** Lacks specific details on *what* has been implemented. Mentioning "all functions and definitions from the datasheet" is not helpful without specifying which ones are done. The summary should clearly state the current state of the driver (e.g., "Initial implementation of the SX126x driver, including basic initialization and register access"). * **Impact:** While mentioning expanded board support is good, the impact section needs to be more concrete. Currently, it's too general. Since the driver isn't functional, many impacts are unknown. Focus on the *current* impact of this *incomplete* PR. For example: * **Impact on user:** None yet, as the driver is not usable. * **Impact on build:** Minimal, adds source files. (Be specific about which files) * **Impact on hardware:** None yet, as the driver is not integrated. * **Impact on documentation:** None yet. Documentation will be added upon completion. * **Security:** Unknown. Will be assessed upon completion. * **Compatibility:** Unknown. Will be addressed upon completion. * **Testing:** Insufficient. "Deep testing has yet to be done" is unacceptable for a PR. Even for work-in-progress, some basic functionality should be demonstrable and testable. The provided "testing logs" are nonexistent. At the very least, demonstrate register read/write functionality. Even if the driver can't transmit/receive yet, *some* testing is required. * **WIP Status:** Marking a PR as WIP is generally discouraged in NuttX (and most open-source projects). It's better to develop on a branch and only submit a PR when it's closer to being ready for review. **Recommendations:** 1. **Focus on a minimal viable implementation:** Instead of trying to implement everything at once, focus on a core set of functionalities (e.g., initialization, register access, basic transmit/receive). Get that working and tested first. 2. **Provide concrete examples:** Show code snippets of the implemented functions, and provide specific register settings used in your test. 3. **Actual testing logs:** Include real logs demonstrating the functionality you *have* implemented. 4. **Break down the work:** Create separate issues for individual features (e.g., "Implement LoRa modulation," "Add SPI communication," "Integrate with NuttX driver framework"). This allows for more focused development and review. 5. **Seek feedback earlier:** Instead of submitting an incomplete PR, consider opening a discussion on the NuttX mailing list or forums to discuss your approach and get feedback before writing a lot of code. By following these recommendations, you can create a PR that meets the NuttX requirements and is more likely to be accepted. -- 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: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
