Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lps33thw: add syscfg to make statistics optional #2990

Merged
merged 1 commit into from May 24, 2023

Conversation

benmccrea
Copy link
Contributor

@benmccrea benmccrea commented May 4, 2023

Adds a syscfg, LPS33THW_STATS, to make use of statistics in the driver optional. The syscfg defaults to 1 (no modification to existing behavior). Some people will not care to use statistics for memory reasons.

I also want to note, the way statistics are currently implemented in the driver prevents it from being used with more than 1 sensor at a time, because lps33thw_init will attempt to register the statistics again the second time it is called. This will result in a sysinit panic:

    /* Initialise the stats entry */
    rc = stats_init(STATS_HDR(g_lps33thwstats),
        STATS_SIZE_INIT_PARMS(g_lps33thwstats, STATS_SIZE_32),
        STATS_NAME_INIT_PARMS(lps33thw_stat_section));
    SYSINIT_PANIC_ASSERT(rc == 0);

    /* Register the entry with the stats registry */
    rc = stats_register(dev->od_name, STATS_HDR(g_lps33thwstats));
    SYSINIT_PANIC_ASSERT(rc == 0);

It would be preferable to change this code so that each sensor has its own statistics, or if all sensors are to share the same statistics, then only call stats_init and stats_register for the first sensor.

Copy link
Contributor

@wes3 wes3 left a comment

Choose a reason for hiding this comment

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

LGTM

@benmccrea benmccrea merged commit ca6e5ec into apache:master May 24, 2023
9 checks passed
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.

None yet

2 participants