examples/buttons: Fix poll mode for button press detection.#1
examples/buttons: Fix poll mode for button press detection.#1Nishant-IFX wants to merge 1 commit intomasterfrom
Conversation
parthitce
left a comment
There was a problem hiding this comment.
Thanks for your efforts @Nishant-IFX
examples/buttons/buttons_main.c
Outdated
| #include <errno.h> | ||
| #include <unistd.h> | ||
| #include <signal.h> | ||
| #include <inttypes.h> |
| static void process_sample(btn_buttonset_t sample, | ||
| btn_buttonset_t *oldsample) | ||
| { | ||
| int i; |
There was a problem hiding this comment.
move under CONFIG_EXAMPLES_BUTTONS_NAMES
| /* Initialize the set to empty, then add the configured button | ||
| * signal. | ||
| */ | ||
|
|
There was a problem hiding this comment.
nit, remove empty lines. applies everywhere
|
Addressed all review comments and updated accordingly. Please review and let me know if any further changes are required. |
| } | ||
|
|
||
| sample = (btn_buttonset_t)value.si_value.sival_int; | ||
| process_sample(sample, &oldsample); |
There was a problem hiding this comment.
CONFIG_EXAMPLES_BUTTONS_NAMES is optional config. If this config is not enabled, then you will run into compilation issues. Ensure, process_sample is called, only when the config is enabled, else print the sample value (as in original code).
examples/buttons/buttons_main.c
Outdated
| } | ||
|
|
||
| /* Ignore the default signal action */ | ||
| { |
There was a problem hiding this comment.
Below section of code related to signal set looks like a redundant one. Is this really needed?
| goto errout_with_fd; | ||
| } | ||
|
|
||
| if (!(fds[0].revents & POLLIN)) |
There was a problem hiding this comment.
Is this to handle spurious wakeup? If so, comment shall be added before continue as /* Spurious wakeup? */
examples/buttons/buttons_main.c
Outdated
| #ifdef CONFIG_EXAMPLES_BUTTONS_NAMES | ||
| process_sample(sample, &oldsample); | ||
| #else | ||
| printf("Sample = %jd\n", (intmax_t)sample); |
There was a problem hiding this comment.
Don't replicate this section of code as this is already part of process_sample. Your first commit is correct and I believe you misunderstood Parthiban's comment. His comment is to move variable declaration "int i" inside CONFIG_EXAMPLES_BUTTONS_NAMES and not to move the entire process_sample inside this macro. Fix this.
Siva-IFX
left a comment
There was a problem hiding this comment.
@parthitce - Compliance check has passed and only CI has failed. Hope this is fine.
|
Added a small follow-up fix: sample values were printing repeatedly in loop. Now printing the sample value only when it changes to avoid redundant output. |
|
@parthitce - please check the latest updates. If everything looks good, could you kindly proceed with the approval? |
Could you please squash the commits? Otherwise good |
d229c96 to
b628c19
Compare
|
@parthitce - Squashed all 5 commits into one and updated the PR. Please have a look. |
- Refactor button handling to properly support poll mode and ensure reliable press/release detection using process_sample() - Rework button state processing structure based on review feedback and align code with NuttX coding standards (include order, formatting, and whitespace cleanup) - Move the loop index variable declaration inside the CONFIG_EXAMPLES_BUTTON_NAMES conditional to ensure variables used by the directive remain properly scoped and avoid configuration-related build warnings - Fix logic that caused sample values to be printed repeatedly in a loop and update output to print only when values change - Final cleanup of sample print logic and minor refactoring for consistency and clarity Tested on: Infineon’s AURIX™ TC397 Evaluation Board: KIT_A2G_TC397_5V_TFT and STM32F4Discovery Signed-off-by: Nishant-IFX <Nishant.Kumar@infineon.com>
b628c19 to
f4bbdea
Compare
Summary
Impact
Testing
I confirm that changes are verified on local setup and works as intended: