drivers/sgp30: use event_timeout, to avoid long ISR code block by fjmolinas · Pull Request #22149 · RIOT-OS/RIOT

fjmolinas · 2026-03-22T19:31:57Z

Contribution description

This avoid reading measurements in ISR context, which will behave differently depending on the ISR context size. To avoid this use events posting to the lowest priority event loop.

I've been away for a while so maybe there is a better way of handling this now?

Testing procedure

I have an nrf52840-mdk with the an sgp30 connected, after waiting for 15s I get readings:

2026-03-22 20:30:08,589 # eCO2 [ppm]: 439
2026-03-22 20:30:08,592 # +-------------------------------------+
2026-03-22 20:30:08,695 # TVOC [ppb]: 4
2026-03-22 20:30:08,696 # eCO2 [ppm]: 439
2026-03-22 20:30:08,706 # +-------------------------------------+
2026-03-22 20:30:08,802 # TVOC [ppb]: 4
2026-03-22 20:30:08,804 # eCO2 [ppm]: 439
2026-03-22 20:30:08,807 # +-------------------------------------+

Without this PR hardfault:

2026-03-22 20:31:28,124 # Device is not yet ready
2026-03-22 20:31:28,226 # TVOC [ppb]: 86
2026-03-22 20:31:28,228 # Context before hardfault:
2026-03-22 20:31:28,234 # main(): This is RIOT! (Version: 2017.10-devel-34759-g66dbcd)
2026-03-22 20:31:28,236 # SGP30 test application
2026-03-22 20:31:28,236 #
2026-03-22 20:31:28,239 # +------------Initializing------------+

Issues/PRs references

None

riot-ci · 2026-03-22T19:58:36Z

Murdock results

❌ FAILED

61ced53 fixup! fixup! fixup! fixup! fixup! drivers/sgp30: use event_timeout, to avoid long ISR code block

Success	Failures	Total	Runtime
1901	0	10281	03m:21s

Artifacts

benpicco · 2026-03-23T14:41:22Z

-    dev->_timer.callback = _read_cb;
-    dev->_timer.arg = dev;
+    event_callback_init(&dev->_event, _read_cb, dev);
+    event_timeout_init(&dev->_timeout, EVENT_PRIO_LOWEST, (event_t *)&dev->_event);


better make the queue configurable

Suggested change

event_timeout_init(&dev->_timeout, EVENT_PRIO_LOWEST, (event_t *)&dev->_event);

event_timeout_init(&dev->_timeout, CONFIG_SGP30_EVENT_QUEUE, (event_t *)&dev->_event);

#ifndef CONFIG_SGP30_EVENT_QUEUE #define CONFIG_SGP30_EVENT_QUEUE EVENT_PRIO_LOWEST #endif

Why EVENT_PRIO_LOWEST instead of EVENT_PRIO_MEDIUM?

None, Is EVENT_PRIO_MEDIUM the best default?

I still think it makes sense to make it configurable

Done, Ok to squash?

…e block

benpicco · 2026-03-28T13:29:36Z

+#ifndef SGP30_STRICT_EVENT_THREAD_QUEUE
+#define SGP30_STRICT_EVENT_THREAD_QUEUE     EVENT_PRIO_MEDIUM


Suggested change

#ifndef SGP30_STRICT_EVENT_THREAD_QUEUE

#define SGP30_STRICT_EVENT_THREAD_QUEUE EVENT_PRIO_MEDIUM

#ifndef CONFIG_SGP30_STRICT_EVENT_THREAD_QUEUE

#define CONFIG_SGP30_STRICT_EVENT_THREAD_QUEUE EVENT_PRIO_MEDIUM

Please also add indentation to the #define here.

No problem, sorry that was not part of the coding-standard when I was active, I didn't know it was added, addressed! Thanks for pointing it out

crasbe · 2026-03-28T13:33:03Z

-    dev->_timer.arg = dev;
+    event_callback_init(&dev->_event, _read_cb, dev);
+    event_timeout_init(&dev->_timeout, SGP30_STRICT_EVENT_THREAD_QUEUE, (event_t *)&dev->_event);
+#error


This would unconditionally error out when MODULE_SGP30_STRICT is defined?

I guess I was testing something locally, and committed by mistake

…ISR code block

…d long ISR code block

…to avoid long ISR code block

fjmolinas · 2026-04-21T16:35:32Z

Ok to squash?

crasbe · 2026-04-23T12:03:13Z

Ok to squash?

Good from my side 👍

drivers/sgp30: use event_timeout, to avoid long ISR code block

583af33

github-actions Bot added the Area: drivers Area: Device drivers label Mar 22, 2026

fjmolinas requested a review from benpicco March 22, 2026 19:32

crasbe added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 22, 2026

crasbe reviewed Mar 22, 2026

View reviewed changes

Comment thread drivers/include/sgp30.h

benpicco reviewed Mar 23, 2026

View reviewed changes

fjmolinas added 2 commits March 24, 2026 19:46

drivers/sgp30: format with uncrustify

46b8dd7

fixup! drivers/sgp30: use event_timeout, to avoid long ISR code block

d75d3f7

crasbe mentioned this pull request Mar 26, 2026

sys/stdio_nimble: Remove periodic callout and update documentation #21206

Open

9 tasks

fixup! fixup! drivers/sgp30: use event_timeout, to avoid long ISR cod…

a516421

…e block

benpicco reviewed Mar 28, 2026

View reviewed changes

Comment thread drivers/include/sgp30.h Outdated

benpicco reviewed Mar 28, 2026

View reviewed changes

crasbe reviewed Mar 28, 2026

View reviewed changes

fjmolinas added 3 commits April 20, 2026 20:50

fixup! fixup! fixup! drivers/sgp30: use event_timeout, to avoid long …

5489038

…ISR code block

fixup! fixup! fixup! fixup! drivers/sgp30: use event_timeout, to avoi…

38f403c

…d long ISR code block

fixup! fixup! fixup! fixup! fixup! drivers/sgp30: use event_timeout, …

61ced53

…to avoid long ISR code block

fjmolinas removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 20, 2026

crasbe added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 23, 2026

	event_timeout_init(&dev->_timeout, EVENT_PRIO_LOWEST, (event_t *)&dev->_event);
	event_timeout_init(&dev->_timeout, CONFIG_SGP30_EVENT_QUEUE, (event_t *)&dev->_event);

		#ifndef SGP30_STRICT_EVENT_THREAD_QUEUE
		#define SGP30_STRICT_EVENT_THREAD_QUEUE EVENT_PRIO_MEDIUM

Conversation

fjmolinas commented Mar 22, 2026

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

riot-ci commented Mar 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Murdock results

Artifacts

Uh oh!

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!

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!

fjmolinas commented Apr 21, 2026

Uh oh!

crasbe commented Apr 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

riot-ci commented Mar 22, 2026 •

edited

Loading