Skip to content

Conversation

@jackwilsdon
Copy link

@jackwilsdon jackwilsdon commented Oct 31, 2024

Description

Fixes building the RP2040 port without configASSERT defined. It seems like no other ports use configASSERT in portmacro.h, so this isn't an issue anywhere else. Normally FreeRTOS.h defines configASSERT, but that can't be included in portmacro.h.

This may be a bit easier to review with whitespace changes ignored: https://github.com/FreeRTOS/FreeRTOS-Kernel/pull/1170/files?w=1

Test Steps

Build the RP2040 port without configASSERT defined.

Checklist:

  • I have tested my changes. No regression in existing tests.
  • I have modified and/or added unit-tests to cover the code changes in this Pull Request.

Related Issue

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@jackwilsdon jackwilsdon requested a review from a team as a code owner October 31, 2024 14:39
@sonarqubecloud
Copy link

@kar-rahul-aws kar-rahul-aws self-requested a review October 31, 2024 16:40
@ActoryOu
Copy link
Member

ActoryOu commented Nov 1, 2024

Hello @jackwilsdon,
Could you help share the compile error log when configASSERT is not defined without this PR?
The fix here is inside a inline function. It should be no issue If the caller also includes the FreeRTOS.h (before portmacro.h).

Thank you.

@jackwilsdon
Copy link
Author

jackwilsdon commented Nov 1, 2024

FreeRTOS.h imports portable.h (which imports portmacro.h) before it defines a default for configASSERT.

In file included from vendor/FreeRTOS-Kernel/include/portable.h:53,
                 from vendor/FreeRTOS-Kernel/include/FreeRTOS.h:107,
                 from src/main.c:1:
vendor/FreeRTOS-Kernel/portable/ThirdParty/GCC/RP2040/include/portmacro.h: In function 'vPortRecursiveLock':
vendor/FreeRTOS-Kernel/portable/ThirdParty/GCC/RP2040/include/portmacro.h:216:5: warning: implicit declaration of function 'configASSERT' [-Wimplicit-function-declaration]
  216 |     configASSERT( ulLockNum < portRTOS_SPINLOCK_COUNT );
      |     ^~~~~~~~~~~~
/usr/lib/gcc/arm-none-eabi/13.2.1/../../../arm-none-eabi/bin/ld: vendor/FreeRTOS-Kernel/tasks.c.obj: in function `vPortRecursiveLock':
vendor/FreeRTOS-Kernel/portable/ThirdParty/GCC/RP2040/include/portmacro.h:216:(.text.prvCheckForRunStateChange+0x30): undefined reference to `configASSERT'

@ActoryOu
Copy link
Member

ActoryOu commented Nov 1, 2024

Hi @jackwilsdon,
May I know the configASSERT definition in your FreeRTOSConfig.h?
If there is no definition, would it be easier to define it to nothing like below if you don't need it?

#define configASSERT( x )

EDIT: FreeRTOS.h includes FreeRTOSConfig.h at line 58. The configASSERT is defined before including portable.h.
EDIT2: Sorry that I missed the default setting part below. Let me bring the discussion back to the team then reply you. But the method I provided here should help you solve this as workaround.

Thank you.

@jackwilsdon
Copy link
Author

jackwilsdon commented Nov 1, 2024

I'm already defining an empty configASSERT as a workaround (which does fix the issue) - it'd be nice if it was consistent with other ports though.

@ActoryOu
Copy link
Member

ActoryOu commented Nov 4, 2024

Hi @jackwilsdon,
Thanks for your patient! We had a discussion about this topic.
We'll create a separate PR to move configASSERT definition above before including portable.h.
Then we don't need to handle this in the port layer anymore!
Will make a note here once the PR is created.

Thank you.

@jackwilsdon jackwilsdon closed this Nov 4, 2024
@jackwilsdon jackwilsdon deleted the fix-undefined-assert-rp2040 branch November 4, 2024 09:49
moninom1 pushed a commit to moninom1/FreeRTOS-Kernel that referenced this pull request Sep 30, 2025
* Add SMP schedule equal priority on target test

* Remove unnecessary config

* Fix spelling format and header

* Code review suggestions

Signed-off-by: Gaurav Aggarwal <[email protected]>

---------

Signed-off-by: Gaurav Aggarwal <[email protected]>
Co-authored-by: Gaurav Aggarwal <[email protected]>
Co-authored-by: Gaurav-Aggarwal-AWS <[email protected]>
Co-authored-by: Rahul Kar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants