Skip to content

Conversation

KozhinovAlexander
Copy link
Contributor

@KozhinovAlexander KozhinovAlexander commented Oct 11, 2025

This commit introduces the DT_INST_STRING_TOKEN_BY_IDX_OR macro, which allows specifying a default value if the indexed string token is not defined.

How-To-Test:

Introduce new property in any .yml file of your choice:

my-prop:
    required: true
    type: string-array
    enum:
      - FOO
      - BAR

Add it to any .dts:

node {
	my-prop = "FOO", "BAR";
	status = "okay";
};

Try to access from .c code an existing index:

#define DT_INST_GET_MY_PROP(inst) DT_INST_STRING_TOKEN_BY_IDX_OR(inst, my_prop, 0, BAZ))

It shall return FOO.

Try to access from .c code a NON-existing index:

#define DT_INST_GET_MY_PROP(inst) DT_INST_STRING_TOKEN_BY_IDX_OR(inst, my_prop, 3, BAZ))

It shall return BAZ.

UPD: Added tests under tests/lib/devicetree/api, thnx @JarmouniA

@KozhinovAlexander KozhinovAlexander changed the title zephyr: devicetree: add DT_INST_STRING_TOKEN_BY_IDX_OR macro DT_INST_STRING_TOKEN_BY_IDX_OR macro Oct 11, 2025
@KozhinovAlexander KozhinovAlexander force-pushed the DT_INST_STRING_TOKEN_BY_IDX_OR branch 4 times, most recently from 537a362 to 82dc587 Compare October 12, 2025 08:08
@KozhinovAlexander
Copy link
Contributor Author

KozhinovAlexander commented Oct 12, 2025

@ALL A question to all here: Are there tests for newly added macros needed? Please provide a linke to similiar tests if possible.

@ttmut
Copy link
Contributor

ttmut commented Oct 12, 2025

I wonder, are drivers allowed to use a value not specified in the binding? Because now it won't be clear that the driver will use BAZ just by looking at the binding and the devicetree node.

@JarmouniA
Copy link
Contributor

@KozhinovAlexander for testing, see tests/lib/devicetree/api

@KozhinovAlexander
Copy link
Contributor Author

KozhinovAlexander commented Oct 12, 2025

I wonder, are drivers allowed to use a value not specified in the binding? Because now it won't be clear that the driver will use BAZ just by looking at the binding and the devicetree node.

This is a good question, targeting user-friendliness and misuse. To be more argumentative, I suggest you to look at DT_INST_PHA_BY_IDX_OR(...) I've used as an already existing similar solution.

Here I am trying to answer your questions:

  1. Misuse: I think - covering misuse shall not be done by macros itself, cause it will end up in a messy code-base. But the code at run- and/or compile-time shall provide valid usage patterns.
  2. User-friendliness: Indeed, the available values are not directly seen from the available values of the string-array definition in .yml file, but same holds for DT_INST_PHA_BY_IDX_OR(...) too. Moreover, the default value covered by OR is a kind of protected value, developers do not want be seen directly. Think about tri-state logic, where the array covers only true/false values, and the third state is reserved for internal use. Same will be here.

@zephyrbot zephyrbot added the area: Tests Issues related to a particular existing or missing test label Oct 12, 2025
@zephyrbot zephyrbot requested a review from nashif October 12, 2025 12:25
@KozhinovAlexander KozhinovAlexander force-pushed the DT_INST_STRING_TOKEN_BY_IDX_OR branch 6 times, most recently from a8e26be to 22dc17b Compare October 12, 2025 13:59
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces the DT_INST_STRING_TOKEN_BY_IDX_OR macro, which provides a safe way to access string tokens from device tree properties with a fallback default value when the specified index is out of range.

  • Adds DT_STRING_TOKEN_BY_IDX_OR and DT_INST_STRING_TOKEN_BY_IDX_OR macros with default value fallback
  • Includes comprehensive test coverage for both in-range and out-of-range index scenarios
  • Updates copyright information to include the contributor

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
include/zephyr/devicetree.h Adds the new macros with documentation and copyright update
tests/lib/devicetree/api/src/main.c Adds test cases to verify macro behavior for valid and invalid indices

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

#define DT_DRV_COMPAT vnd_string_array_token
ZTEST(devicetree_api, test_string_idx_token)
{
/* The enum has 7 values in total - thus invalid idx starts with 16 */
Copy link

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

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

The comment states invalid indices start with 16, but the test uses index 42 for out-of-range testing. Either update the comment to reflect the actual test values or explain why 42 was chosen instead of 16.

Suggested change
/* The enum has 7 values in total - thus invalid idx starts with 16 */
/* The enum has 16 values in total (indices 0-15), so invalid indices start at 16.
* The test uses index 42 for out-of-range testing to ensure it is well beyond the valid range.
*/

Copilot uses AI. Check for mistakes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The number 42 is famously known as "The Answer to the Ultimate Question of Life, the Universe, and Everything"

Copy link
Member

Choose a reason for hiding this comment

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

the robot has a point though that 16 is probably better for test to avoid off by one errors

@KozhinovAlexander KozhinovAlexander force-pushed the DT_INST_STRING_TOKEN_BY_IDX_OR branch from 22dc17b to a6a296a Compare October 12, 2025 19:48
@KozhinovAlexander
Copy link
Contributor Author

@mbolivar Would it be possible for you to take a look here?

This commit introduces the DT_INST_STRING_TOKEN_BY_IDX_OR
and DT_INST_STRING_TOKEN_BY_IDX macros, which allows
specifying a default value if the indexed string token is not
defined.

Signed-off-by: Alexander Kozhinov <[email protected]>
add tests for DT_STRING_TOKEN_BY_IDX_OR macro
add tests for DT_INST_STRING_TOKEN_BY_IDX_OR macro

Signed-off-by: Alexander Kozhinov <[email protected]>
@KozhinovAlexander KozhinovAlexander force-pushed the DT_INST_STRING_TOKEN_BY_IDX_OR branch from a6a296a to dd76b5c Compare October 14, 2025 18:57
Copy link

@KozhinovAlexander
Copy link
Contributor Author

@nashif I think - there is no reaction from assignee, please take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Devicetree area: Tests Issues related to a particular existing or missing test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants