pbio/drv/button: Simplify reading.

laurensvalk · laurensvalk · commit 1e297526900f · 2025-07-18T15:28:12.000+02:00
Most platforms have no way to report errors for reading the button. And even if we did, there is no way to handle it. It is better to assume that nothing is pressed if the buttons are broken.
diff --git a/lib/pbio/drv/button/button_gpio.c b/lib/pbio/drv/button/button_gpio.c
@@ -52,15 +52,12 @@ static pbio_button_flags_t pbdrv_button_gpio_read(void) {
     return flags;
 }
 
-
-pbio_error_t pbdrv_button_is_pressed(pbio_button_flags_t *pressed) {
+pbio_button_flags_t pbdrv_button_get_pressed(void) {
     #if PBDRV_CONFIG_BUTTON_GPIO_DEBOUNCE
-    *pressed = pbdrv_button_state;
+    return pbdrv_button_state;
     #else
-    *pressed = pbdrv_button_gpio_read();
+    return pbdrv_button_gpio_read();
     #endif
-
-    return PBIO_SUCCESS;
 }
 
 #if PBDRV_CONFIG_BUTTON_GPIO_DEBOUNCE
diff --git a/lib/pbio/drv/button/button_nxt.c b/lib/pbio/drv/button/button_nxt.c
@@ -14,26 +14,26 @@
 void pbdrv_button_init(void) {
 }
 
-pbio_error_t pbdrv_button_is_pressed(pbio_button_flags_t *pressed) {
+pbio_button_flags_t pbdrv_button_get_pressed(void) {
 
     nx_avr_button_t button = nx_avr_get_button();
 
-    *pressed = 0;
+    pbio_button_flags_t pressed = 0;
 
     if (button == BUTTON_OK) {
-        *pressed |= PBIO_BUTTON_CENTER;
+        pressed |= PBIO_BUTTON_CENTER;
     }
     if (button == BUTTON_LEFT) {
-        *pressed |= PBIO_BUTTON_LEFT;
+        pressed |= PBIO_BUTTON_LEFT;
     }
     if (button == BUTTON_RIGHT) {
-        *pressed |= PBIO_BUTTON_RIGHT;
+        pressed |= PBIO_BUTTON_RIGHT;
     }
     if (button == BUTTON_CANCEL) {
-        *pressed |= PBIO_BUTTON_DOWN;
+        pressed |= PBIO_BUTTON_DOWN;
     }
 
-    return PBIO_SUCCESS;
+    return pressed;
 }
 
 #endif // PBDRV_CONFIG_BUTTON_NXT
diff --git a/lib/pbio/drv/button/button_resistor_ladder.c b/lib/pbio/drv/button/button_resistor_ladder.c
@@ -14,43 +14,43 @@
 void pbdrv_button_init(void) {
 }
 
-pbio_error_t pbdrv_button_is_pressed(pbio_button_flags_t *pressed) {
+pbio_button_flags_t pbdrv_button_get_pressed(void) {
     pbdrv_resistor_ladder_ch_flags_t flags;
     pbio_error_t err;
 
-    *pressed = 0;
+    pbio_button_flags_t pressed = 0;
 
     // REVISIT: For now, this is hard-coded for SPIKE Prime. If more platforms
     // that use this are added, a resistor ladder flags to buttons map will
     // need to be added to platform data.
 
     err = pbdrv_resistor_ladder_get(0, &flags);
     if (err != PBIO_SUCCESS) {
-        return err;
+        return pressed;
     }
 
     if (flags & PBDRV_RESISTOR_LADDER_CH_1) {
-        *pressed |= PBIO_BUTTON_CENTER;
+        pressed |= PBIO_BUTTON_CENTER;
     }
 
     err = pbdrv_resistor_ladder_get(1, &flags);
     if (err != PBIO_SUCCESS) {
-        return err;
+        return pressed;
     }
 
     if (flags & PBDRV_RESISTOR_LADDER_CH_0) {
-        *pressed |= PBIO_BUTTON_LEFT;
+        pressed |= PBIO_BUTTON_LEFT;
     }
 
     if (flags & PBDRV_RESISTOR_LADDER_CH_1) {
-        *pressed |= PBIO_BUTTON_RIGHT;
+        pressed |= PBIO_BUTTON_RIGHT;
     }
 
     if (flags & PBDRV_RESISTOR_LADDER_CH_2) {
-        *pressed |= PBIO_BUTTON_RIGHT_UP; // Bluetooth
+        pressed |= PBIO_BUTTON_RIGHT_UP; // Bluetooth
     }
 
-    return PBIO_SUCCESS;
+    return pressed;
 }
 
 #endif // PBDRV_CONFIG_BUTTON_RESISTOR_LADDER
diff --git a/lib/pbio/drv/button/button_test.c b/lib/pbio/drv/button/button_test.c
@@ -19,9 +19,8 @@ void pbio_test_button_set_pressed(pbio_button_flags_t flags) {
 void pbdrv_button_init(void) {
 }
 
-pbio_error_t pbdrv_button_is_pressed(pbio_button_flags_t *pressed) {
-    *pressed = pbio_test_button_flags;
-    return PBIO_SUCCESS;
+pbio_button_flags_t pbdrv_button_get_pressed(void) {
+    return pbio_test_button_flags;
 }
 
 #endif // PBDRV_CONFIG_BUTTON_TEST
diff --git a/lib/pbio/include/pbdrv/button.h b/lib/pbio/include/pbdrv/button.h
@@ -22,18 +22,15 @@
 
 /**
  * Get bitmask indicating currently pressed buttons.
- * @param [out] pressed     Bitmask indicating which buttons are pressed
- * @return                  ::PBIO_SUCCESS if the call was successful,
- *                          ::PBIO_ERROR_NO_DEV if port is valid but a device with buttons is not connected
- *                          ::PBIO_ERROR_IO if there was an I/O error
+ *
+ * @return                  Bitmask indicating which buttons are pressed
  */
-pbio_error_t pbdrv_button_is_pressed(pbio_button_flags_t *pressed);
+pbio_button_flags_t pbdrv_button_get_pressed(void);
 
 #else
 
-static inline pbio_error_t pbdrv_button_is_pressed(pbio_button_flags_t *pressed) {
-    *pressed = 0;
-    return PBIO_ERROR_NOT_SUPPORTED;
+static inline pbio_button_flags_t pbdrv_button_get_pressed(void) {
+    return 0;
 }
 
 #endif
diff --git a/lib/pbio/include/pbio/button.h b/lib/pbio/include/pbio/button.h
@@ -100,30 +100,9 @@ typedef enum _pbio_button_flags_t {
 
 } pbio_button_flags_t;
 
-// Generic button state getter
-typedef pbio_error_t (*pbio_button_is_pressed_func_t)(pbio_button_flags_t *pressed);
-
-#ifdef DOXYGEN
-
-/**
- * Get bitmask indicating currently pressed buttons.
- * @param [out] pressed     Bitmask indicating which buttons are pressed
- * @return                  ::PBIO_SUCCESS if the call was successful,
- *                          ::PBIO_ERROR_NO_DEV if port is valid but a device with buttons is not connected
- *                          ::PBIO_ERROR_IO if there was an I/O error
- */
-pbio_error_t pbio_button_is_pressed(pbio_button_flags_t *pressed);
-
-#else // DOXYGEN
-
-// calling pbdrv function directly for efficiency
-#define pbio_button_is_pressed pbdrv_button_is_pressed
-
-// include for pbdrv_button_is_pressed needs to be called after pbio_button_flags_t is defined
+// include for pbdrv_button_get_pressed needs to be called after pbio_button_flags_t is defined
 #include <pbdrv/button.h>
 
-#endif // DOXYGEN
-
 #endif // _PBIO_BUTTON_H_
 
 /** @} */
diff --git a/lib/pbio/platform/ev3/platform.c b/lib/pbio/platform/ev3/platform.c
@@ -735,9 +735,7 @@ void SystemInit(void) {
  * as a very convenient way to power off the EV3 for fast iteration.
  */
 void lazy_poweroff_hook(void) {
-    pbio_button_flags_t flags;
-    pbio_error_t err = pbio_button_is_pressed(&flags);
-    if (err == PBIO_SUCCESS && (flags & PBIO_BUTTON_LEFT_UP)) {
+    if (pbdrv_button_get_pressed() & PBIO_BUTTON_LEFT_UP) {
         pbdrv_reset_power_off();
         return;
     }
diff --git a/lib/pbio/sys/hmi.c b/lib/pbio/sys/hmi.c
@@ -183,30 +183,28 @@ void pbsys_hmi_handle_status_change(pbsys_status_change_t event, pbio_pybricks_s
  * This is called periodically to update the current HMI state.
  */
 void pbsys_hmi_poll(void) {
-    pbio_button_flags_t btn;
+    pbio_button_flags_t btn = pbdrv_button_get_pressed();
 
-    if (pbio_button_is_pressed(&btn) == PBIO_SUCCESS) {
-        if (btn & PBIO_BUTTON_CENTER) {
-            pbsys_status_set(PBIO_PYBRICKS_STATUS_POWER_BUTTON_PRESSED);
-            update_program_run_button_wait_state(true);
+    if (btn & PBIO_BUTTON_CENTER) {
+        pbsys_status_set(PBIO_PYBRICKS_STATUS_POWER_BUTTON_PRESSED);
+        update_program_run_button_wait_state(true);
 
-            // power off when button is held down for 2 seconds
-            if (pbsys_status_test_debounce(PBIO_PYBRICKS_STATUS_POWER_BUTTON_PRESSED, true, 2000)) {
-                pbsys_status_set(PBIO_PYBRICKS_STATUS_SHUTDOWN_REQUEST);
-            }
-        } else {
-            pbsys_status_clear(PBIO_PYBRICKS_STATUS_POWER_BUTTON_PRESSED);
-            update_program_run_button_wait_state(false);
+        // power off when button is held down for 2 seconds
+        if (pbsys_status_test_debounce(PBIO_PYBRICKS_STATUS_POWER_BUTTON_PRESSED, true, 2000)) {
+            pbsys_status_set(PBIO_PYBRICKS_STATUS_SHUTDOWN_REQUEST);
         }
+    } else {
+        pbsys_status_clear(PBIO_PYBRICKS_STATUS_POWER_BUTTON_PRESSED);
+        update_program_run_button_wait_state(false);
+    }
 
-        #if PBSYS_CONFIG_BLUETOOTH_TOGGLE
-        update_bluetooth_button_wait_state(btn & PBSYS_CONFIG_BLUETOOTH_TOGGLE_BUTTON);
-        #endif // PBSYS_CONFIG_BLUETOOTH_TOGGLE
+    #if PBSYS_CONFIG_BLUETOOTH_TOGGLE
+    update_bluetooth_button_wait_state(btn & PBSYS_CONFIG_BLUETOOTH_TOGGLE_BUTTON);
+    #endif // PBSYS_CONFIG_BLUETOOTH_TOGGLE
 
-        #if PBSYS_CONFIG_HMI_NUM_SLOTS
-        update_left_right_button_wait_state(btn & PBIO_BUTTON_LEFT, btn & PBIO_BUTTON_RIGHT);
-        #endif // PBSYS_CONFIG_HMI_NUM_SLOTS
-    }
+    #if PBSYS_CONFIG_HMI_NUM_SLOTS
+    update_left_right_button_wait_state(btn & PBIO_BUTTON_LEFT, btn & PBIO_BUTTON_RIGHT);
+    #endif // PBSYS_CONFIG_HMI_NUM_SLOTS
 
     pbsys_status_light_poll();
 }
diff --git a/lib/pbio/sys/program_stop.c b/lib/pbio/sys/program_stop.c
@@ -63,8 +63,7 @@ void pbsys_program_stop_poll(void) {
         return;
     }
 
-    pbio_button_flags_t btn;
-    pbio_button_is_pressed(&btn);
+    pbio_button_flags_t btn = pbdrv_button_get_pressed();
 
     if (!stop_buttons) {
         return;
diff --git a/pybricks/hubs/pb_type_ev3brick.c b/pybricks/hubs/pb_type_ev3brick.c
@@ -24,8 +24,7 @@ typedef struct _hubs_EV3Brick_obj_t {
 } hubs_EV3Brick_obj_t;
 
 static mp_obj_t pb_type_ev3brick_button_pressed(void) {
-    pbio_button_flags_t flags;
-    pb_assert(pbio_button_is_pressed(&flags));
+    pbio_button_flags_t flags = pbdrv_button_get_pressed();
     mp_obj_t pressed[5];
     size_t num = 0;
     if (flags & PBIO_BUTTON_LEFT) {
diff --git a/pybricks/hubs/pb_type_nxtbrick.c b/pybricks/hubs/pb_type_nxtbrick.c
@@ -26,8 +26,7 @@ typedef struct _hubs_NXTBrick_obj_t {
 } hubs_NXTBrick_obj_t;
 
 static mp_obj_t pb_type_nxtbrick_button_pressed(void) {
-    pbio_button_flags_t flags;
-    pb_assert(pbio_button_is_pressed(&flags));
+    pbio_button_flags_t flags = pbdrv_button_get_pressed();
     mp_obj_t pressed[4];
     size_t num = 0;
     if (flags & PBIO_BUTTON_LEFT) {
diff --git a/pybricks/hubs/pb_type_primehub.c b/pybricks/hubs/pb_type_primehub.c
@@ -42,8 +42,7 @@ typedef struct _hubs_PrimeHub_obj_t {
 } hubs_PrimeHub_obj_t;
 
 static mp_obj_t pb_type_primehub_button_pressed(void) {
-    pbio_button_flags_t flags;
-    pb_assert(pbio_button_is_pressed(&flags));
+    pbio_button_flags_t flags = pbdrv_button_get_pressed();
     mp_obj_t pressed[4];
     size_t num = 0;
     if (flags & PBIO_BUTTON_LEFT) {
diff --git a/pybricks/parameters/pb_type_button.c b/pybricks/parameters/pb_type_button.c
@@ -154,8 +154,7 @@ pbio_button_flags_t pb_type_button_get_button_flag(mp_obj_t obj) {
  * Common button pressed function for single button hubs.
  */
 mp_obj_t pb_type_button_pressed_hub_single_button(void) {
-    pbio_button_flags_t flags;
-    pb_assert(pbio_button_is_pressed(&flags));
+    pbio_button_flags_t flags = pbdrv_button_get_pressed();
     mp_obj_t buttons[] = { pb_type_button_new(MP_QSTR_CENTER) };
 
     #if MICROPY_PY_BUILTINS_SET
diff --git a/pybricks/tools/pb_module_tools.c b/pybricks/tools/pb_module_tools.c
@@ -275,12 +275,10 @@ static pbio_button_flags_t pb_module_tools_hub_menu_wait_for_press(bool press) {
     // This function should only be used in a blocking context.
     pb_module_tools_assert_blocking();
 
-    pbio_error_t err;
     pbio_button_flags_t btn;
-    while ((err = pbio_button_is_pressed(&btn)) == PBIO_SUCCESS && ((bool)btn) == !press) {
+    while ((bool)(btn = pbdrv_button_get_pressed()) == !press) {
         MICROPY_EVENT_POLL_HOOK;
     }
-    pb_assert(err);
     return btn;
 }
 

Original file line number	Diff line number	Diff line change
`@@ -14,26 +14,26 @@`
`14`	`14`	`void pbdrv_button_init(void) {`
`15`	`15`	`}`
`16`	`16`
`17`		`-pbio_error_t pbdrv_button_is_pressed(pbio_button_flags_t *pressed) {`
	`17`	`+pbio_button_flags_t pbdrv_button_get_pressed(void) {`
`18`	`18`
`19`	`19`	`nx_avr_button_t button = nx_avr_get_button();`
`20`	`20`
`21`		`- *pressed = 0;`
	`21`	`+ pbio_button_flags_t pressed = 0;`
`22`	`22`
`23`	`23`	`if (button == BUTTON_OK) {`
`24`		`- *pressed \|= PBIO_BUTTON_CENTER;`
	`24`	`+ pressed \|= PBIO_BUTTON_CENTER;`
`25`	`25`	`}`
`26`	`26`	`if (button == BUTTON_LEFT) {`
`27`		`- *pressed \|= PBIO_BUTTON_LEFT;`
	`27`	`+ pressed \|= PBIO_BUTTON_LEFT;`
`28`	`28`	`}`
`29`	`29`	`if (button == BUTTON_RIGHT) {`
`30`		`- *pressed \|= PBIO_BUTTON_RIGHT;`
	`30`	`+ pressed \|= PBIO_BUTTON_RIGHT;`
`31`	`31`	`}`
`32`	`32`	`if (button == BUTTON_CANCEL) {`
`33`		`- *pressed \|= PBIO_BUTTON_DOWN;`
	`33`	`+ pressed \|= PBIO_BUTTON_DOWN;`
`34`	`34`	`}`
`35`	`35`
`36`		`- return PBIO_SUCCESS;`
	`36`	`+ return pressed;`
`37`	`37`	`}`
`38`	`38`
`39`	`39`	`#endif // PBDRV_CONFIG_BUTTON_NXT`
Original file line number	Diff line number	Diff line change
`@@ -14,43 +14,43 @@`
`14`	`14`	`void pbdrv_button_init(void) {`
`15`	`15`	`}`
`16`	`16`
`17`		`-pbio_error_t pbdrv_button_is_pressed(pbio_button_flags_t *pressed) {`
	`17`	`+pbio_button_flags_t pbdrv_button_get_pressed(void) {`
`18`	`18`	`pbdrv_resistor_ladder_ch_flags_t flags;`
`19`	`19`	`pbio_error_t err;`
`20`	`20`
`21`		`- *pressed = 0;`
	`21`	`+ pbio_button_flags_t pressed = 0;`
`22`	`22`
`23`	`23`	`// REVISIT: For now, this is hard-coded for SPIKE Prime. If more platforms`
`24`	`24`	`// that use this are added, a resistor ladder flags to buttons map will`
`25`	`25`	`// need to be added to platform data.`
`26`	`26`
`27`	`27`	`err = pbdrv_resistor_ladder_get(0, &flags);`
`28`	`28`	`if (err != PBIO_SUCCESS) {`
`29`		`- return err;`
	`29`	`+ return pressed;`
`30`	`30`	`}`
`31`	`31`
`32`	`32`	`if (flags & PBDRV_RESISTOR_LADDER_CH_1) {`
`33`		`- *pressed \|= PBIO_BUTTON_CENTER;`
	`33`	`+ pressed \|= PBIO_BUTTON_CENTER;`
`34`	`34`	`}`
`35`	`35`
`36`	`36`	`err = pbdrv_resistor_ladder_get(1, &flags);`
`37`	`37`	`if (err != PBIO_SUCCESS) {`
`38`		`- return err;`
	`38`	`+ return pressed;`
`39`	`39`	`}`
`40`	`40`
`41`	`41`	`if (flags & PBDRV_RESISTOR_LADDER_CH_0) {`
`42`		`- *pressed \|= PBIO_BUTTON_LEFT;`
	`42`	`+ pressed \|= PBIO_BUTTON_LEFT;`
`43`	`43`	`}`
`44`	`44`
`45`	`45`	`if (flags & PBDRV_RESISTOR_LADDER_CH_1) {`
`46`		`- *pressed \|= PBIO_BUTTON_RIGHT;`
	`46`	`+ pressed \|= PBIO_BUTTON_RIGHT;`
`47`	`47`	`}`
`48`	`48`
`49`	`49`	`if (flags & PBDRV_RESISTOR_LADDER_CH_2) {`
`50`		`- *pressed \|= PBIO_BUTTON_RIGHT_UP; // Bluetooth`
	`50`	`+ pressed \|= PBIO_BUTTON_RIGHT_UP; // Bluetooth`
`51`	`51`	`}`
`52`	`52`
`53`		`- return PBIO_SUCCESS;`
	`53`	`+ return pressed;`
`54`	`54`	`}`
`55`	`55`
`56`	`56`	`#endif // PBDRV_CONFIG_BUTTON_RESISTOR_LADDER`
Original file line number	Diff line number	Diff line change
`@@ -19,9 +19,8 @@ void pbio_test_button_set_pressed(pbio_button_flags_t flags) {`
`19`	`19`	`void pbdrv_button_init(void) {`
`20`	`20`	`}`
`21`	`21`
`22`		`-pbio_error_t pbdrv_button_is_pressed(pbio_button_flags_t *pressed) {`
`23`		`- *pressed = pbio_test_button_flags;`
`24`		`- return PBIO_SUCCESS;`
	`22`	`+pbio_button_flags_t pbdrv_button_get_pressed(void) {`
	`23`	`+ return pbio_test_button_flags;`
`25`	`24`	`}`
`26`	`25`
`27`	`26`	`#endif // PBDRV_CONFIG_BUTTON_TEST`
Original file line number	Diff line number	Diff line change
`@@ -63,8 +63,7 @@ void pbsys_program_stop_poll(void) {`
`63`	`63`	`return;`
`64`	`64`	`}`
`65`	`65`
`66`		`- pbio_button_flags_t btn;`
`67`		`- pbio_button_is_pressed(&btn);`
	`66`	`+ pbio_button_flags_t btn = pbdrv_button_get_pressed();`
`68`	`67`
`69`	`68`	`if (!stop_buttons) {`
`70`	`69`	`return;`