pbio/sys: Begin simplifying light_matrix.

There are three files named light_matrix.h, and several structures with a single field defined in one of the other ones. Some were directly including from the ../src/ directory. There were also funcs types, but they had only one function, and they always use a led_array.

In practice, there will be only one light matrix. It has only one function to set one pixel on one led array.

This simplifies it by cutting out some of the abstractions. And it still allows having more than one light matrix if we ever need it. We can remove the pbio/sys/light_matrix abstraction in the next commit.

Author: laurensvalk

lib/pbio/include/pbio/light_matrix.h

@@ -20,17 +20,22 @@ typedef struct _pbio_light_matrix_t pbio_light_matrix_t;
 
 #if PBIO_CONFIG_LIGHT_MATRIX
 
+pbio_error_t pbio_light_matrix_get_dev(uint8_t index, uint8_t size, pbio_light_matrix_t **light_matrix);
 uint8_t pbio_light_matrix_get_size(pbio_light_matrix_t *light_matrix);
 void pbio_light_matrix_set_orientation(pbio_light_matrix_t *light_matrix, pbio_geometry_side_t up_side);
 pbio_error_t pbio_light_matrix_clear(pbio_light_matrix_t *light_matrix);
 pbio_error_t pbio_light_matrix_set_rows(pbio_light_matrix_t *light_matrix, const uint8_t *rows);
-pbio_error_t pbio_light_matrix_set_pixel(pbio_light_matrix_t *light_matrix, uint8_t row, uint8_t col, uint8_t brightness);
+pbio_error_t pbio_light_matrix_set_pixel(pbio_light_matrix_t *light_matrix, uint8_t row, uint8_t col, uint8_t brightness, bool clear_animation);
 pbio_error_t pbio_light_matrix_set_image(pbio_light_matrix_t *light_matrix, const uint8_t *image);
 void pbio_light_matrix_start_animation(pbio_light_matrix_t *light_matrix, const uint8_t *cells, uint8_t num_cells, uint16_t interval);
 void pbio_light_matrix_stop_animation(pbio_light_matrix_t *light_matrix);
 
 #else // PBIO_CONFIG_LIGHT_MATRIX
 
+static inline pbio_error_t pbio_light_matrix_get_dev(uint8_t index, uint8_t size, pbio_light_matrix_t **light_matrix) {
+    return PBIO_ERROR_NOT_SUPPORTED;
+}
+
 static inline uint8_t pbio_light_matrix_get_size(pbio_light_matrix_t *light_matrix) {
     return 0;
 }
@@ -46,7 +51,7 @@ static inline pbio_error_t pbio_light_matrix_set_rows(pbio_light_matrix_t *light
     return PBIO_ERROR_NOT_SUPPORTED;
 }
 
-static inline pbio_error_t pbio_light_matrix_set_pixel(pbio_light_matrix_t *light_matrix, uint8_t row, uint8_t col, uint8_t brightness) {
+static inline pbio_error_t pbio_light_matrix_set_pixel(pbio_light_matrix_t *light_matrix, uint8_t row, uint8_t col, uint8_t brightness, bool clear_animation) {
     return PBIO_ERROR_NOT_SUPPORTED;
 }
 

lib/pbio/platform/ev3/pbioconfig.h

@@ -8,7 +8,6 @@
 #define PBIO_CONFIG_IMAGE                   (1)
 #define PBIO_CONFIG_IMU                     (0)
 #define PBIO_CONFIG_LIGHT                   (1)
-#define PBIO_CONFIG_LIGHT_MATRIX            (1)
 #define PBIO_CONFIG_LOGGER                  (1)
 #define PBIO_CONFIG_MOTOR_PROCESS           (1)
 #define PBIO_CONFIG_PORT                    (1)

lib/pbio/platform/prime_hub/pbioconfig.h

@@ -9,6 +9,7 @@
 #define PBIO_CONFIG_LIGHT                   (1)
 #define PBIO_CONFIG_LOGGER                  (1)
 #define PBIO_CONFIG_LIGHT_MATRIX            (1)
+#define PBIO_CONFIG_LIGHT_MATRIX_NUM_DEV    (1)
 #define PBIO_CONFIG_MOTOR_PROCESS           (1)
 #define PBIO_CONFIG_PORT                    (1)
 #define PBIO_CONFIG_PORT_NUM_DEV            (6)

lib/pbio/src/light/light_matrix.c

@@ -14,6 +14,41 @@
 #include "animation.h"
 #include "light_matrix.h"
 
+static pbio_light_matrix_t light_matrices[PBIO_CONFIG_LIGHT_MATRIX_NUM_DEV];
+
+/**
+ * Gets and initializes the light matrix.
+ *
+ * Skips initialization if already initialized.
+ *
+ * @param [in]   index           Device index.
+ * @param [in]   size            The size of the square light matrix.
+ * @param [out]  light_matrix    The light matrix
+ * @return   ::PBIO_SUCCESS if device is available and initialization succeeds or completed previously.
+ *           ::PBIO_ERROR_NO_DEV if this index is invalid.
+ */
+pbio_error_t pbio_light_matrix_get_dev(uint8_t index, uint8_t size, pbio_light_matrix_t **light_matrix) {
+    if (index >= PBIO_CONFIG_LIGHT_MATRIX_NUM_DEV) {
+        return PBIO_ERROR_NO_DEV;
+    }
+    pbio_light_matrix_t *dev = &light_matrices[index];
+    *light_matrix = dev;
+
+    if (dev->led_array_dev) {
+        // Already configured.
+        return PBIO_SUCCESS;
+    }
+
+    pbio_error_t err = pbdrv_led_array_get_dev(index, &dev->led_array_dev);
+    if (err != PBIO_SUCCESS) {
+        return err;
+    }
+
+    dev->size = size;
+    pbio_light_animation_init(&dev->animation, NULL);
+    return PBIO_SUCCESS;
+}
+
 /**
  * Sets the pixel to a given brightness.
  *
@@ -54,23 +89,12 @@ static pbio_error_t _pbio_light_matrix_set_pixel(pbio_light_matrix_t *light_matr
         }
     }
 
-    // Set the pixel brightness
-    return light_matrix->funcs->set_pixel(light_matrix, row, col, brightness);
-}
+    if (!light_matrix->led_array_dev) {
+        return PBIO_ERROR_NO_DEV;
+    }
 
-/**
- * Initializes the required fields in a ::pbio_light_matrix_t.
- *
- * This function must be called before using the ::pbio_light_matrix_t.
- *
- * @param [in]  light_matrix  The struct to initialize.
- * @param [in]  size        The size of the light matrix.
- * @param [in]  funcs       The instance-specific callback functions.
- */
-void pbio_light_matrix_init(pbio_light_matrix_t *light_matrix, uint8_t size, const pbio_light_matrix_funcs_t *funcs) {
-    light_matrix->size = size;
-    light_matrix->funcs = funcs;
-    pbio_light_animation_init(&light_matrix->animation, NULL);
+    // Set the pixel brightness
+    return pbdrv_led_array_set_brightness(light_matrix->led_array_dev, row * light_matrix->size + col, brightness);
 }
 
 /**
@@ -173,12 +197,13 @@ pbio_error_t pbio_light_matrix_set_rows(pbio_light_matrix_t *light_matrix, const
  * @param [in]  row         Row index (0 to size-1)
  * @param [in]  col         Column index (0 to size-1)
  * @param [in]  brightness  Brightness (0 to 100)
+ * @param [in]  clear_animation Whether to clear the matrix if an animation was active.
  * @return                  ::PBIO_SUCCESS on success or an
  *                          implementation-specific error on failure.
  */
-pbio_error_t pbio_light_matrix_set_pixel(pbio_light_matrix_t *light_matrix, uint8_t row, uint8_t col, uint8_t brightness) {
+pbio_error_t pbio_light_matrix_set_pixel(pbio_light_matrix_t *light_matrix, uint8_t row, uint8_t col, uint8_t brightness, bool clear_animation) {
 
-    if (pbio_light_animation_is_started(&light_matrix->animation)) {
+    if (clear_animation && pbio_light_animation_is_started(&light_matrix->animation)) {
         pbio_light_matrix_clear(light_matrix);
     }
     return _pbio_light_matrix_set_pixel(light_matrix, row, col, brightness);

lib/pbio/src/light/light_matrix.h

@@ -3,6 +3,8 @@
 
 #include <stdint.h>
 
+#include <pbdrv/led.h>
+
 #include <pbio/error.h>
 #include <pbio/light_matrix.h>
 
@@ -11,25 +13,9 @@
 #ifndef _PBIO_LIGHT_LIGHT_MATRIX_H_
 #define _PBIO_LIGHT_LIGHT_MATRIX_H_
 
-/** Implementation-specific callbacks for a light matrix. */
-typedef struct {
-    /**
-     * Sets the light at @p row, @p col to @p brightness.
-     *
-     * @param [in]  light_matrix  The light matrix instance.
-     * @param [in]  row         The row index (0 to size-1).
-     * @param [in]  col         The column index (0 to size-1).
-     * @param [in]  brightness  The apparent brightness (0 to 100).
-     * @return                  Success/failure of the operation.
-     */
-    pbio_error_t (*set_pixel)(pbio_light_matrix_t *light_matrix, uint8_t row, uint8_t col, uint8_t brightness);
-} pbio_light_matrix_funcs_t;
-
 struct _pbio_light_matrix_t {
     /** Animation instance for background animation. */
     pbio_light_animation_t animation;
-    /** Implementation specific callback functions. */
-    const pbio_light_matrix_funcs_t *funcs;
     /** Animation cell data. */
     const uint8_t *animation_cells;
     /** The number of cells in @p animation_cells */
@@ -42,8 +28,8 @@ struct _pbio_light_matrix_t {
     uint8_t size;
     /** Orientation of the matrix: which side is "up". */
     pbio_geometry_side_t up_side;
+    /** The driver for this light matrix. */
+    pbdrv_led_array_dev_t *led_array_dev;
 };
 
-void pbio_light_matrix_init(pbio_light_matrix_t *light_matrix, uint8_t size, const pbio_light_matrix_funcs_t *funcs);
-
 #endif // _PBIO_LIGHT_LIGHT_MATRIX_H_

lib/pbio/sys/light_matrix.c

@@ -23,40 +23,8 @@
 
 #if PBSYS_CONFIG_HUB_LIGHT_MATRIX
 
-typedef struct {
-    /** Struct for PBIO light matrix implementation. */
-    pbio_light_matrix_t light_matrix;
-} pbsys_hub_light_matrix_t;
-
-static pbsys_hub_light_matrix_t pbsys_hub_light_matrix_instance;
-
-/** The hub built-in light matrix instance. */
-pbio_light_matrix_t *pbsys_hub_light_matrix = &pbsys_hub_light_matrix_instance.light_matrix;
-
-static pbio_error_t pbsys_hub_light_matrix_set_pixel(pbio_light_matrix_t *light_matrix, uint8_t row, uint8_t col, uint8_t brightness) {
-    #if PBSYS_CONFIG_HUB_LIGHT_MATRIX_LED_ARRAY
-    // REVISIT: currently hub light matrix is hard-coded as LED array at index 0
-    // on all platforms
-    pbdrv_led_array_dev_t *array;
-    if (pbdrv_led_array_get_dev(0, &array) == PBIO_SUCCESS) {
-        return pbdrv_led_array_set_brightness(array, row * light_matrix->size + col, brightness);
-    }
-    #elif PBSYS_CONFIG_HUB_LIGHT_MATRIX_DISPLAY
-    pbio_image_t *display = pbdrv_display_get_image();
-    uint8_t value = brightness * (pbdrv_display_get_max_value() + 1) / 100;
-    const uint32_t size = PBDRV_CONFIG_DISPLAY_NUM_ROWS / PBSYS_CONFIG_HMI_NUM_SLOTS;
-    const uint32_t width = size * 4 / 5;
-    const uint32_t offset = (PBDRV_CONFIG_DISPLAY_NUM_COLS - (PBSYS_CONFIG_HMI_NUM_SLOTS * size)) / 2;
-    pbio_image_fill_rect(display, col * size + offset, row * size, width, width, value);
-    pbdrv_display_update();
-    return PBIO_SUCCESS;
-    #endif
-    return PBIO_ERROR_NOT_SUPPORTED;
-}
-
-static const pbio_light_matrix_funcs_t pbsys_hub_light_matrix_funcs = {
-    .set_pixel = pbsys_hub_light_matrix_set_pixel,
-};
+// revisit, we will drop this in the next commit
+pbio_light_matrix_t *pbsys_hub_light_matrix;
 
 /**
  * Displays the idle UI. Has a square stop sign and selected slot on bottom row.
@@ -70,7 +38,7 @@ static void pbsys_hub_light_matrix_show_idle_ui(uint8_t brightness) {
             #if PBSYS_CONFIG_HMI_NUM_SLOTS
             is_on |= (r == 4 && c == pbsys_status_get_selected_slot());
             #endif
-            pbsys_hub_light_matrix_set_pixel(pbsys_hub_light_matrix, r, c, is_on ? brightness : 0);
+            pbio_light_matrix_set_pixel(pbsys_hub_light_matrix, r, c, is_on ? brightness : 0, false);
         }
     }
 }
@@ -109,7 +77,7 @@ static void pbsys_hub_light_matrix_start_power_animation(void) {
 }
 
 void pbsys_hub_light_matrix_init(void) {
-    pbio_light_matrix_init(pbsys_hub_light_matrix, 5, &pbsys_hub_light_matrix_funcs);
+    pbio_light_matrix_get_dev(0, 5, &pbsys_hub_light_matrix);
     pbsys_hub_light_matrix_start_power_animation();
 }
 
@@ -124,7 +92,7 @@ void pbsys_hub_light_matrix_deinit(void) {
 static void pbsys_hub_light_matrix_user_program_animation_clear(void) {
     for (uint8_t r = 0; r < 3; r++) {
         for (uint8_t c = 1; c < 4; c++) {
-            pbsys_hub_light_matrix_set_pixel(pbsys_hub_light_matrix, r, c, 0);
+            pbio_light_matrix_set_pixel(pbsys_hub_light_matrix, r, c, 0, true);
         }
     }
 }
@@ -144,7 +112,7 @@ static uint32_t pbsys_hub_light_matrix_user_program_animation_next(pbio_light_an
         uint8_t brightness = offset > 200 ? 0 : (offset < 100 ? offset : 200 - offset);
 
         // Set the brightness for this pixel
-        pbsys_hub_light_matrix_set_pixel(pbsys_hub_light_matrix, indexes[i] / 5, indexes[i] % 5, brightness);
+        pbio_light_matrix_set_pixel(pbsys_hub_light_matrix, indexes[i] / 5, indexes[i] % 5, brightness, false);
     }
     // This increment controls the speed of the pattern
     cycle += 9;

lib/pbio/test/src/test_light_matrix.c

@@ -58,17 +58,17 @@ static PT_THREAD(test_light_matrix(struct pt *pt)) {
 
     // set pixel should only set one pixel
     test_light_matrix_reset();
-    tt_want_uint_op(pbio_light_matrix_set_pixel(&test_light_matrix, 0, 0, 100), ==, PBIO_SUCCESS);
+    tt_want_uint_op(pbio_light_matrix_set_pixel(&test_light_matrix, 0, 0, 100, true), ==, PBIO_SUCCESS);
     tt_want_light_matrix_data(100, 0, 0, 0, 0, 0, 0, 0, 0);
 
     tt_want_uint_op(pbio_light_matrix_set_pixel(&test_light_matrix,
-        MATRIX_SIZE - 1, MATRIX_SIZE - 1, 100), ==, PBIO_SUCCESS);
+        MATRIX_SIZE - 1, MATRIX_SIZE - 1, 100, true), ==, PBIO_SUCCESS);
     tt_want_light_matrix_data(100, 0, 0, 0, 0, 0, 0, 0, 100);
 
     // out of bounds checking
-    tt_want_uint_op(pbio_light_matrix_set_pixel(&test_light_matrix, MATRIX_SIZE, 0, 100), ==, PBIO_SUCCESS);
+    tt_want_uint_op(pbio_light_matrix_set_pixel(&test_light_matrix, MATRIX_SIZE, 0, 100, true), ==, PBIO_SUCCESS);
     tt_want_light_matrix_data(100, 0, 0, 0, 0, 0, 0, 0, 100);
-    tt_want_uint_op(pbio_light_matrix_set_pixel(&test_light_matrix, 0, MATRIX_SIZE, 100), ==, PBIO_SUCCESS);
+    tt_want_uint_op(pbio_light_matrix_set_pixel(&test_light_matrix, 0, MATRIX_SIZE, 100, true), ==, PBIO_SUCCESS);
     tt_want_light_matrix_data(100, 0, 0, 0, 0, 0, 0, 0, 100);
 
     // bitwise mapping

pybricks/common/pb_type_lightmatrix.c

@@ -160,7 +160,7 @@ static mp_obj_t common_LightMatrix_on(size_t n_args, const mp_obj_t *pos_args, m
 
     for (uint8_t i = 0; i < size; i++) {
         for (uint8_t j = 0; j < size; j++) {
-            pb_assert(pbio_light_matrix_set_pixel(self->light_matrix, i, j, brightness));
+            pb_assert(pbio_light_matrix_set_pixel(self->light_matrix, i, j, brightness, true));
         }
     }
 
@@ -218,7 +218,7 @@ void pb_type_LightMatrix_display_number(pbio_light_matrix_t *light_matrix, mp_ob
 
     // Display one faint dot in the middle to indicate negative
     if (negative) {
-        pb_assert(pbio_light_matrix_set_pixel(light_matrix, 2, 2, 50));
+        pb_assert(pbio_light_matrix_set_pixel(light_matrix, 2, 2, 50, true));
     }
 }
 
@@ -276,7 +276,7 @@ static mp_obj_t common_LightMatrix_pixel(size_t n_args, const mp_obj_t *pos_args
         PB_ARG_DEFAULT_INT(brightness, 100));
 
     // Set pixel at the given brightness
-    pb_assert(pbio_light_matrix_set_pixel(self->light_matrix, pb_obj_get_int(row_in), pb_obj_get_int(column_in), pb_obj_get_pct(brightness_in)));
+    pb_assert(pbio_light_matrix_set_pixel(self->light_matrix, pb_obj_get_int(row_in), pb_obj_get_int(column_in), pb_obj_get_pct(brightness_in), true));
 
     return mp_const_none;
 }