You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "hujun260 (via GitHub)" <gi...@apache.org> on 2025/01/07 04:03:17 UTC
[PR] fix compile error [nuttx]
hujun260 opened a new pull request, #15441:
URL: https://github.com/apache/nuttx/pull/15441
## Summary
fix compile error
Configuration/Tool: sim/segger
In file included from segger/SystemView/SEGGER/SEGGER_SYSVIEW.h:65,
from segger/SystemView/SEGGER/SEGGER_SYSVIEW_Int.h:64,
from segger/SystemView/SEGGER/SEGGER_SYSVIEW.c:146:
segger/SystemView/SEGGER/SEGGER_SYSVIEW.c: In function '_VPrintHost': Error: segger/SystemView/SEGGER/SEGGER_SYSVIEW_ConfDefaults.h:557:51: error: implicit declaration of function 'SEGGER_RTT_LOCK'; did you mean 'SEGGER_RTT_Read'? [-Werror=implicit-function-declaration]
557 | #define SEGGER_SYSVIEW_LOCK() SEGGER_RTT_LOCK()
| ^~~~~~~~~~~~~~~
segger/SystemView/SEGGER/SEGGER_SYSVIEW.c:387:35: note: in expansion of macro 'SEGGER_SYSVIEW_LOCK'
387 | #define RECORD_START(PacketSize) SEGGER_SYSVIEW_LOCK(); \
| ^~~~~~~~~~~~~~~~~~~
segger/SystemView/SEGGER/SEGGER_SYSVIEW.c:1005:5: note: in expansion of macro 'RECORD_START'
1005 | RECORD_START(SEGGER_SYSVIEW_INFO_SIZE + SEGGER_SYSVIEW_MAX_STRING_LEN + 2 * SEGGER_SYSVIEW_QUANTA_U32 + SEGGER_SYSVIEW_MAX_ARGUMENTS * SEGGER_SYSVIEW_QUANTA_U32);
| ^~~~~~~~~~~~
Error: segger/SystemView/SEGGER/SEGGER_SYSVIEW_ConfDefaults.h:570:51: error: implicit declaration of function 'SEGGER_RTT_UNLOCK'; did you mean 'SEGGER_RTT_Init'? [-Werror=implicit-function-declaration]
570 | #define SEGGER_SYSVIEW_UNLOCK() SEGGER_RTT_UNLOCK()
| ^~~~~~~~~~~~~~~~~
segger/SystemView/SEGGER/SEGGER_SYSVIEW.c:390:35: note: in expansion of macro 'SEGGER_SYSVIEW_UNLOCK'
390 | #define RECORD_END() SEGGER_SYSVIEW_UNLOCK()
| ^~~~~~~~~~~~~~~~~~~~~
segger/SystemView/SEGGER/SEGGER_SYSVIEW.c:1015:5: note: in expansion of macro 'RECORD_END'
1015 | RECORD_END();
*Note: Please adhere to [Contributing Guidelines](https://github.com/apache/nuttx/blob/master/CONTRIBUTING.md).*
## Impact
none
## Testing
ci
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix compile error [nuttx]
Posted by "nuttxpr (via GitHub)" <gi...@apache.org>.
nuttxpr commented on PR #15441:
URL: https://github.com/apache/nuttx/pull/15441#issuecomment-2574352254
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
No, this PR does not meet the NuttX requirements. Here's why and how to fix it:
**Missing Information:**
* **Summary:** While it mentions a compile error, it lacks crucial details:
* **Why is the change necessary?** Simply stating "fix compile error" is insufficient. Explain *why* the error occurs. Was a dependency updated? Was there a logic flaw?
* **What functional part is changed?** Be specific. Is it related to SystemView integration? RTT? A specific driver?
* **How does the change work?** What code was added/modified to resolve the error? Just providing the error message isn't helpful.
* **Impact:** Even if you believe there's no user-facing impact, explicitly state "NO" for each impact category. If the fix involves including new header files or changes linking, the build *might* be impacted. Review carefully.
* **Testing:** "ci" is inadequate. Provide details about your *local* testing. Did you run the simulator? On what host? What commands did you use to build and test? Include relevant snippets of the build output (success/failure) *before* and *after* the change. CI results are supplemental to your local testing, not a replacement for it.
**How to Improve the PR:**
1. **Explain the root cause:** Why was `SEGGER_RTT_LOCK` undeclared? Was a header file missing? Was a configuration option incorrect?
2. **Describe the solution:** What did you change to fix the error? Did you add a header file? Did you modify a configuration setting? Provide a clear, concise explanation of the code changes. Example: "Added `#include <SEGGER_RTT.h>` to `SEGGER_SYSVIEW.c` to resolve the missing declaration of `SEGGER_RTT_LOCK` and `SEGGER_RTT_UNLOCK`."
3. **Detailed Impact Assessment:** Go through each impact category and explicitly state "NO" or "YES" with a brief explanation if "YES". Example: "Impact on build: YES. Requires the `SEGGER_RTT` library to be linked."
4. **Thorough Testing Information:**
* **Build Host:** Specify your OS, CPU architecture, and compiler version. Example: "Linux x86_64, GCC 12.2.0"
* **Target:** Specify the target architecture, board, and configuration. Example: "sim:nsh"
* **Logs:** Provide *actual* build logs and test output before and after the change, even if it's just a successful build. Focus on the relevant parts. Don't just paste entire log files.
**Example of a better Summary:**
"Fixes a compile error in the SystemView integration for the `sim/segger` configuration caused by missing declarations for `SEGGER_RTT_LOCK` and `SEGGER_RTT_UNLOCK`. The error occurred because `SEGGER_RTT.h` was not included. This change adds the necessary include directive to `SEGGER_SYSVIEW.c` to resolve the issue."
By providing more complete information, you'll make it easier for reviewers to understand and approve your PR.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix compile error [nuttx]
Posted by "anchao (via GitHub)" <gi...@apache.org>.
anchao commented on code in PR #15441:
URL: https://github.com/apache/nuttx/pull/15441#discussion_r1905451372
##########
drivers/segger/note_sysview.c:
##########
@@ -92,6 +92,8 @@ static void note_sysview_vprintf(FAR struct note_driver_s *drv, uintptr_t ip,
* Private Data
****************************************************************************/
+spinlock_t g_segger_lock = SP_UNLOCKED;
Review Comment:
private data?
##########
drivers/segger/config/SEGGER_RTT_Conf.h:
##########
@@ -77,6 +77,20 @@
#define SEGGER_RTT_MEMCPY_USE_BYTELOOP 0
+/* RTT lock configuration */
+
+#ifndef __ASSEMBLY__
+extern spinlock_t g_segger_lock;
Review Comment:
move to begin
##########
drivers/segger/config/SEGGER_RTT_Conf.h:
##########
@@ -77,6 +77,20 @@
#define SEGGER_RTT_MEMCPY_USE_BYTELOOP 0
+/* RTT lock configuration */
+
+#ifndef __ASSEMBLY__
+extern spinlock_t g_segger_lock;
+#endif
+
+/* Lock RTT (nestable) (i.e. disable interrupts) */
+
+#define SEGGER_RTT_LOCK() irqstate_t __flags = spin_lock_irqsave(&g_segger_lock)
Review Comment:
Do you make changes without verifying them? Please run the relevant tests of Segger Sysview and upload the test screenshots
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix compile error [nuttx]
Posted by "anchao (via GitHub)" <gi...@apache.org>.
anchao commented on code in PR #15441:
URL: https://github.com/apache/nuttx/pull/15441#discussion_r1905462225
##########
drivers/segger/config/SEGGER_RTT_Conf.h:
##########
@@ -77,6 +77,20 @@
#define SEGGER_RTT_MEMCPY_USE_BYTELOOP 0
+/* RTT lock configuration */
+
+#ifndef __ASSEMBLY__
+extern spinlock_t g_segger_lock;
+#endif
+
+/* Lock RTT (nestable) (i.e. disable interrupts) */
+
+#define SEGGER_RTT_LOCK() irqstate_t __flags = spin_lock_irqsave(&g_segger_lock)
Review Comment:
If you can't provide test result, then revert the change in #15413
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix compile error [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #15441:
URL: https://github.com/apache/nuttx/pull/15441
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix compile error [nuttx]
Posted by "anchao (via GitHub)" <gi...@apache.org>.
anchao commented on PR #15441:
URL: https://github.com/apache/nuttx/pull/15441#issuecomment-2575302862
@hujun260 If you continue to make changes to the kernel code without any testing, I will have to reject all future submissions for merging into the community !
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix compile error [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #15441:
URL: https://github.com/apache/nuttx/pull/15441#discussion_r1904880663
##########
drivers/segger/config/SEGGER_RTT_Conf.h:
##########
@@ -77,6 +77,16 @@
#define SEGGER_RTT_MEMCPY_USE_BYTELOOP 0
+/* RTT lock configuration */
+
+/* Lock RTT (nestable) (i.e. disable interrupts) */
+
+#define SEGGER_RTT_LOCK() irqstate_t __flags = up_irq_save()
Review Comment:
how to handle smp case?
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix compile error [nuttx]
Posted by "hujun260 (via GitHub)" <gi...@apache.org>.
hujun260 commented on code in PR #15441:
URL: https://github.com/apache/nuttx/pull/15441#discussion_r1904966738
##########
drivers/segger/config/SEGGER_RTT_Conf.h:
##########
@@ -77,6 +77,16 @@
#define SEGGER_RTT_MEMCPY_USE_BYTELOOP 0
+/* RTT lock configuration */
+
+/* Lock RTT (nestable) (i.e. disable interrupts) */
+
+#define SEGGER_RTT_LOCK() irqstate_t __flags = up_irq_save()
Review Comment:
done
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org