test: new prototypical semi-automatic test. by csanchezdll · Pull Request #215 · candle-usb/candleLight_fw

csanchezdll · 2026-01-20T17:18:11Z

This is the test script I used to detect (and check the fix) of #207 and #211.

The README.md should explain everything. Having to build "instrumented" versions of the FW is somewhat clumsy, but is the best way I found of forcing the runtime conditions I wanted to test. Using a breakpoint with and returning inside GDB would be another option, but it is more intrusive in terms of runtime timing.

If you run this before #209, it will fail right away, as the device will HardFault (trying to receive into a NULL buffer). If you run if on a branch without #212 (but with #209, otherwise the more frequent one hides the least frequent one) it will pass, usually, but running in a loop makes it eventually fail (in my system, it takes 100-200 iterations).

No failures running this on main up to 2500 iterations.

fenugrec

some clever bash work in there.
Have not tested, just skimmed; with a few comments.

fenugrec · 2026-01-20T22:29:40Z

test/README.md

+
+The script on this directory performs a limited test for
+device functionality. It is meant to be run with a device
+under test (DUT) connected thought USB and JTAG, and


JTAG/SWD probably ?

Any way your GDB server of choice can connect, really, but yes as these are STM devices openocd and JTAG/SWD is correct. Changing.

fenugrec · 2026-01-20T22:30:03Z

test/README.md

+
+The test is sligtly intrusive, because it temporarily
+suspends the FW capability of sending frames to the host
+thorugh USB. This is done by building a special "test"


fenugrec · 2026-01-20T22:30:50Z

test/README.md

+	* GDB server (openocd, most likely) running
+
+2. Start by building the special test version of the
+   code


" (append _test_fw suffix to your usual target name) "

fenugrec · 2026-01-20T22:35:57Z

test/test.sh

+candump_dut=$tmpdir/candump_dut
+candump_aux=$tmpdir/candump_aux
+
+mkfifo $gdb_in


I don't see a corresponding close/delete for this fifo ?

It is created inside $tmpdir which is removed at the end of the script, so no need for explicit removal (same as any other file).
One problem of the script as it is is when it timeouts (which is actually a common failure mode, as the DUT HardFaults) it hangs and when interrupting it it leaves a lot of slate subprocess and $tmpdir is silently kept. I am updating the script so it includes a timeout, removes subprocesses, and chimes about keeping the $tmpdir for examination on failures.

btw: you can use trap to automatically execute a bash function on a signal, i.e. exit of the script.

trap_exit_handler() { echo 'insert cleanup stuff here' } trap 'trap_exit_handler' 0 1 15

Yup, I know, that is what I am doing. This script started as a quick & dirty thing to catch the bug, that is why the "failing" exit was not so taken care of.

fenugrec · 2026-01-20T22:36:59Z

test/test.sh

+fi
+
+sudo ip link set $interface_aux down
+sudo ip link set $interface_aux type can bitrate 500000


would running at 1M help to trigger the bug faster ?

Probably not. The bug about the double buffer triggers right away, regardless of speed. And the bug about an IRQ interrupting "prepare" depends on the timing between the IRQ and main loop... which is affected by timing in general but not necessarily that way.
No harm on raising it, 1Mbps being the highest standard CAN speed. Let me check the bug reproduces, just in case, and I'll update.

Actually, the interrupted "prepare" bug (#211) seems to happen slightly less frequently at 1Mbps. Just chance, IMHO, but I have kept 500kbps by default and added a -b switch to select any desired bitrate.

marckleinebudde · 2026-01-21T09:31:14Z

src/usbd_gs_can.c

+#ifdef USBD_TEST
+volatile int skip_send = 0;
+#endif
 void USBD_GS_CAN_SendToHost(USBD_HandleTypeDef *pdev)
 {
+#ifdef USBD_TEST
+	if (skip_send)
+		return;
+#endif
+


Suggested change

#ifdef USBD_TEST

volatile int skip_send = 0;

#endif

void USBD_GS_CAN_SendToHost(USBD_HandleTypeDef *pdev)

{

#ifdef USBD_TEST

if (skip_send)

return;

#endif

volatile int skip_send;

void USBD_GS_CAN_SendToHost(USBD_HandleTypeDef *pdev)

{

if (IS_ENABLED(USBD_TEST) && skip_send)

return;

Changing. Just thinking... if we are making skip_test not being ifdef'ed away on non-test FWs, then why using USBD_TEST at all? We could just predicate the early return on skip_test alone, as it is never set by normal code, and this way we could avoid having a separate "test" version of the binaries.
I did not want to do it right away to avoid "polluting" the FW with test features. What do you prefer?

grrr - my comment was here 2 times, then I deleted 1. Now it's gone :/ - here we go again:

No need to init as 0, as it's a global variable.
With the IS_ENABLED() helper, we can get rid of the ifdef.
Usually I'd make it static, but not sure, if the debugger can still find it.

Changing. Just thinking... if we are making skip_test not being ifdef'ed away on non-test FWs, then why using USBD_TEST at all?

If it's not ifdef'ed, the compiler will always compile the code, but the optimizer would remove it for the normal firmware. This has the benefit, that the syntax is always checked, even if the code is not in the resulting binary.

I am pretty sure static does not affect visibility in GDB, but I am checking. About init, you are totally right. I'll use IS_ENABLED()... or should we just remove USBD_TEST altogether and avoid splitting the FW binaries?

I'm fine with the extra binary.

I am not sure if the optimizer removing skip_test holds for volatile vars... but I do not think it is a big deal even if it is kept, and I agree with runtime checks being better than conditional compilation for syntax checking.

candleLight_fw: mainline:

Memory region Used Size Region Size %age Used FLASH: 17716 B 128 KB 13.52% RAM: 4512 B 16 KB 27.54% create and sign dfu bin file: candleLight_fw text data bss dec hex filename 17696 84 4448 22228 56d4 candleLight_fw

candleLight_fw: your branch + ifdef replaced by IS_ENABLED():

Memory region Used Size Region Size %age Used FLASH: 17716 B 128 KB 13.52% RAM: 4516 B 16 KB 27.56% create and sign dfu bin file: candleLight_fw text data bss dec hex filename 17696 84 4452 22232 56d8 candleLight_fw

Text stays the same, but BSS grows.

You're right the variable is still in the BSS, but the code in USBD_GS_CAN_SendToHost() has been optimized out.

candleLight_test_fw: your branch + ifdef replaced by IS_ENABLED():

Memory region Used Size Region Size %age Used FLASH: 17748 B 128 KB 13.54% RAM: 4516 B 16 KB 27.56% create and sign dfu bin file: candleLight_test_fw text data bss dec hex filename 17728 84 4452 22264 56f8 candleLight_test_fw

csanchezdll · 2026-01-21T12:52:31Z

@marckleinebudde I think I addresses all your comments. I added separate commits for clarity, I can squash them before merging if you prefer a shorter git history.

csanchezdll · 2026-01-22T16:04:25Z

test/test.sh

+    then
+	printf "Keeping temporary directory in %s\n" $tmpdir
+    else
+	rm -rf $tmpdit


fenugrec · 2026-01-29T18:13:50Z

In light of recent issues, maybe adding a few tests with cangen + cansequence would make sense ? if not in this script, maybe some other not too far.

csanchezdll · 2026-01-29T18:15:09Z

I can add it :)

test: new prototypical semi-automatic test.

53ccfa3

csanchezdll mentioned this pull request Jan 20, 2026

gs_usb: reserve pre-allocated RX buffers. #209

Merged

fenugrec reviewed Jan 20, 2026

View reviewed changes

marckleinebudde reviewed Jan 21, 2026

View reviewed changes

csanchezdll added 3 commits January 21, 2026 13:49

test: avoid conditional compilation.

9053752

test: exit cleanly when interrupted + doc fixes.

f292516

test: allow selecting CAN bitrate.

3d607a5

csanchezdll commented Jan 22, 2026

View reviewed changes

test/test.sh

then

printf "Keeping temporary directory in %s\n" $tmpdir

else

rm -rf $tmpdit

Copy link

Contributor Author

csanchezdll Jan 22, 2026

Choose a reason for hiding this comment

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

tmpdir

Conversation

csanchezdll commented Jan 20, 2026

Uh oh!

fenugrec left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marckleinebudde Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marckleinebudde Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marckleinebudde Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

csanchezdll commented Jan 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fenugrec commented Jan 29, 2026

Uh oh!

csanchezdll commented Jan 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

marckleinebudde Jan 21, 2026 •

edited

Loading

marckleinebudde Jan 21, 2026 •

edited

Loading

marckleinebudde Jan 21, 2026 •

edited

Loading