Skip to content

Conversation

lemrey
Copy link
Contributor

@lemrey lemrey commented Jun 15, 2025

A dynamic queue implementation based on Zephyr's k_queue. The library has a compatibility mode to export Zephyr's k_queue API, but it also exposes its own bm_queue API so that bare metal libraries and the application can avoid including zephyr/kernel.h and using a k_ prefixed (kernel's) API.

Note, I couldn't write a unit test for this given that UNITY depends on MULTITHREADING so I slapped the calls in a sample to check that it compiled, but haven't done much testing beyond that.

I based this on Jamie's #155 .

@lemrey lemrey requested review from nordicjm and a team June 15, 2025 15:01
Copy link
Contributor

@nordicjm nordicjm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure why this needs heap? Tried it in firmware loader and got this:

E: ***** BUS FAULT *****
E:   Precise data bus error
E:   BFAR Address: 0x3fe24234
E: r0/a1:  0x20006678  r1/a2:  0x000000bf  r2/a3:  0x0000001f
E: r3/a4:  0x3fe24234 r12/ip:  0x00000000 r14/lr:  0x000abeff
E:  xpsr:  0x8900002d
E: Faulting instruction address (r15/pc): 0x000ae9b0
E: >>> ZEPHYR FATAL ERROR 25: Unknown error on CPU 0
E: Fault during interrupt handling

E: Halting system

@nordicjm
Copy link
Contributor

nordicjm commented Jun 17, 2025

upped heap to 32KiB (is 0 with the zephyr queue system) and get the same result:

E: ***** BUS FAULT *****
E:   Precise data bus error
E:   BFAR Address: 0x3fe24234
E: r0/a1:  0x20006678  r1/a2:  0x000000bf  r2/a3:  0x0000001f
E: r3/a4:  0x3fe24234 r12/ip:  0x00000000 r14/lr:  0x000abeff
E:  xpsr:  0x8900002d
E: Faulting instruction address (r15/pc): 0x000ae9b0
E: >>> ZEPHYR FATAL ERROR 25: Unknown error on CPU 0
E: Fault during interrupt handling

->

addr2line -e firmware_loader/zephyr/zephyr.elf 0x000ae9b0
/tmp/bb/modules/lib/zcbor/src/zcbor_encode.c:51 (discriminator 2)

so seems to be something wrong with this library, after switching back to the zephyr one the application runs flawlessly

@lemrey lemrey force-pushed the bm_queue-kcompat branch from 27f1117 to 9a10362 Compare June 18, 2025 10:01
@lemrey lemrey requested a review from a team as a code owner June 18, 2025 10:01
@lemrey lemrey force-pushed the bm_queue-kcompat branch from 9a10362 to 1dc5f0f Compare June 18, 2025 10:05
A dynamic queue implementation based on Zephyr's k_queue.
The library has a compatibility mode to export Zephyr's k_queue API,
but it also exposes its own bm_queue API set so that bare metal
libraries and application can avoid including zephyr/kernel.h
and using a k_ prefixed (kernel's) API.

Signed-off-by: Emanuele Di Santo <[email protected]>
@lemrey lemrey force-pushed the bm_queue-kcompat branch from 1dc5f0f to 9a4d31a Compare June 18, 2025 10:09
@lemrey lemrey requested a review from a team as a code owner June 18, 2025 10:09
@lemrey
Copy link
Contributor Author

lemrey commented Jun 18, 2025

Thanks for giving it a try! I had a made a silly mistake, you should be able to get a bit further with it now.

k_queue_alloc_append(&k, &i);
k_queue_alloc_prepend(&k, &i);
k_queue_append_list(&k, NULL, NULL);
k_queue_merge_slist(&k, NULL);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note. Had to comment out this line when running the sample.

This function crashes because NULL is not a valid value for input argument list. sys_slist_is_empty(list) blindly dereferences list. This seems to be the same functionality as k_queue_merge_slist in zephyr/kernel/queue.c, it's just the usage here that is wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I just added those to make sure they compiled :)

@lemrey
Copy link
Contributor Author

lemrey commented Jun 19, 2025

And indeed it did not need the heap, so I have removed that requirement. In case there is none then allocations just fail, but that's how it's supposed to work.

@nordicjm
Copy link
Contributor

Thanks for giving it a try! I had a made a silly mistake, you should be able to get a bit further with it now.

Seems to work now

* Copyright (c) 2016, Wind River Systems, Inc.
* Copyright (c) 2025, Nordic Semiconductor ASA
*
* SPDX-License-Identifier: Apache-2.0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the license, since this file is mostly copied from kernel.h and also modified, the modifications must be marked clearly. A link to the the source file in the commit message would also be helpful.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is marked * Copyright (c) 2025, Nordic Semiconductor ASA

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are our modifications also covered by Apache-2.0 license, then, and not the Nordic 5-clause license?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can't relicense the file, it has to remain under the original license

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.

4 participants