Rework based on Dan's review

10 files changed, 56 insertions, 18 deletions

diff --git a/shared-bindings/displayio/Display.c b/shared-bindings/displayio/Display.c
index ded4119ca..1eb1943b8 100644
--- a/shared-bindings/displayio/Display.c
+++ b/shared-bindings/displayio/Display.c
@@ -218,16 +218,22 @@ STATIC mp_obj_t displayio_display_obj_show(mp_obj_t self_in, mp_obj_t group_in)
 }
 MP_DEFINE_CONST_FUN_OBJ_2(displayio_display_show_obj, displayio_display_obj_show);
 
-//|   .. method:: refresh(*, target_frames_per_second=None, minimum_frames_per_second=1)
+//|   .. method:: refresh(*, target_frames_per_second=60, minimum_frames_per_second=1)
 //|
-//|     When auto refresh is off, waits for the target frame rate and then refreshes the display. If
-//|     the call is too late for the given target frame rate, then the refresh returns immediately
-//|     without updating the screen to hopefully help getting caught up. If the current frame rate
-//|     is below the minimum frame rate, then an exception will be raised.
+//|     When auto refresh is off, waits for the target frame rate and then refreshes the display,
+//|     returning True. If the call has taken too long since the last refresh call for the given
+//|     target frame rate, then the refresh returns False immediately without updating the screen to
+//|     hopefully help getting caught up.
+//|
+//|     If the time since the last successful refresh is below the minimum frame rate, then an
+//|     exception will be raised. Set minimum_frames_per_second to 0 to disable.
 //|
 //|     When auto refresh is on, updates the display immediately. (The display will also update
 //|     without calls to this.)
 //|
+//|   :param int target_frames_per_second: How many times a second `refresh` should be called and the screen updated.
+//|   :param int minimum_frames_per_second: The minimum number of times the screen should be updated per second.
+//|
 STATIC mp_obj_t displayio_display_obj_refresh(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
     enum { ARG_target_frames_per_second, ARG_minimum_frames_per_second };
     static const mp_arg_t allowed_args[] = {
@@ -238,8 +244,12 @@ STATIC mp_obj_t displayio_display_obj_refresh(size_t n_args, const mp_obj_t *pos
     mp_arg_parse_all(n_args - 1, pos_args + 1, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
 
     displayio_display_obj_t *self = native_display(pos_args[0]);
-    common_hal_displayio_display_refresh(self, 1000 / args[ARG_target_frames_per_second].u_int, 1000 / args[ARG_minimum_frames_per_second].u_int);
-    return mp_const_none;
+    uint32_t maximum_ms_per_real_frame = 0xffffffff;
+    mp_int_t minimum_frames_per_second = args[ARG_minimum_frames_per_second].u_int;
+    if (minimum_frames_per_second > 0) {
+        maximum_ms_per_real_frame = 1000 / minimum_frames_per_second;
+    }
+    return mp_obj_new_bool(common_hal_displayio_display_refresh(self, 1000 / args[ARG_target_frames_per_second].u_int, maximum_ms_per_real_frame));
 }
 MP_DEFINE_CONST_FUN_OBJ_KW(displayio_display_refresh_obj, 1, displayio_display_obj_refresh);
 
diff --git a/shared-bindings/displayio/Display.h b/shared-bindings/displayio/Display.h
index 06c848d0d..2d7b68c4e 100644
--- a/shared-bindings/displayio/Display.h
+++ b/shared-bindings/displayio/Display.h
@@ -51,7 +51,7 @@ bool common_hal_displayio_display_show(displayio_display_obj_t* self,
                                        displayio_group_t* root_group);
 
 // Times in ms.
-void common_hal_displayio_display_refresh(displayio_display_obj_t* self, uint32_t target_frame_time, uint32_t maximum_frame_time);
+bool common_hal_displayio_display_refresh(displayio_display_obj_t* self, uint32_t target_ms_per_frame, uint32_t maximum_ms_per_real_frame);
 
 bool common_hal_displayio_display_get_auto_refresh(displayio_display_obj_t* self);
 void common_hal_displayio_display_set_auto_refresh(displayio_display_obj_t* self, bool auto_refresh);
diff --git a/shared-bindings/displayio/EPaperDisplay.c b/shared-bindings/displayio/EPaperDisplay.c
index 5d0d8ac94..81e06f82f 100644
--- a/shared-bindings/displayio/EPaperDisplay.c
+++ b/shared-bindings/displayio/EPaperDisplay.c
@@ -55,7 +55,7 @@
 //|
 //|   Create a EPaperDisplay object on the given display bus (`displayio.FourWire` or `displayio.ParallelBus`).
 //|
-//|   The ``start_sequence`` and ``stop_sequence`` bitpacked to minimize the ram impact. Every
+//|   The ``start_sequence`` and ``stop_sequence`` are bitpacked to minimize the ram impact. Every
 //|   command begins with a command byte followed by a byte to determine the parameter count and if
 //|   a delay is need after. When the top bit of the second byte is 1, the next byte will be the
 //|   delay time in milliseconds. The remaining 7 bits are the parameter count excluding any delay
diff --git a/shared-bindings/displayio/FourWire.c b/shared-bindings/displayio/FourWire.c
index c69623344..51203d460 100644
--- a/shared-bindings/displayio/FourWire.c
+++ b/shared-bindings/displayio/FourWire.c
@@ -146,8 +146,12 @@ STATIC mp_obj_t displayio_fourwire_obj_send(size_t n_args, const mp_obj_t *pos_a
     while (!common_hal_displayio_fourwire_begin_transaction(self)) {
         RUN_BACKGROUND_TASKS;
     }
-    common_hal_displayio_fourwire_send(self, true, args[ARG_toggle_every_byte].u_bool, &command, 1);
-    common_hal_displayio_fourwire_send(self, false, args[ARG_toggle_every_byte].u_bool, ((uint8_t*) bufinfo.buf), bufinfo.len);
+    display_chip_select_behavior_t chip_select = CHIP_SELECT_UNTOUCHED;
+    if (args[ARG_toggle_every_byte].u_bool) {
+        chip_select = CHIP_SELECT_TOGGLE_EVERY_BYTE;
+    }
+    common_hal_displayio_fourwire_send(self, DISPLAY_COMMAND, chip_select, &command, 1);
+    common_hal_displayio_fourwire_send(self, DISPLAY_DATA, chip_select, ((uint8_t*) bufinfo.buf), bufinfo.len);
     common_hal_displayio_fourwire_end_transaction(self);
 
     return mp_const_none;
diff --git a/shared-bindings/displayio/FourWire.h b/shared-bindings/displayio/FourWire.h
index 9c71d63cc..d0935f063 100644
--- a/shared-bindings/displayio/FourWire.h
+++ b/shared-bindings/displayio/FourWire.h
@@ -28,6 +28,8 @@
 #define MICROPY_INCLUDED_SHARED_BINDINGS_DISPLAYBUSIO_FOURWIRE_H
 
 #include "shared-module/displayio/FourWire.h"
+
+#include "shared-bindings/displayio/__init__.h"
 #include "common-hal/microcontroller/Pin.h"
 
 #include "shared-module/displayio/Group.h"
@@ -45,7 +47,7 @@ bool common_hal_displayio_fourwire_bus_free(mp_obj_t self);
 
 bool common_hal_displayio_fourwire_begin_transaction(mp_obj_t self);
 
-void common_hal_displayio_fourwire_send(mp_obj_t self, bool command, bool toggle_every_byte, uint8_t *data, uint32_t data_length);
+void common_hal_displayio_fourwire_send(mp_obj_t self, display_byte_type_t byte_type, display_chip_select_behavior_t chip_select, uint8_t *data, uint32_t data_length);
 
 void common_hal_displayio_fourwire_end_transaction(mp_obj_t self);
 
diff --git a/shared-bindings/displayio/I2CDisplay.c b/shared-bindings/displayio/I2CDisplay.c
index 4de5e83ef..4339d182f 100644
--- a/shared-bindings/displayio/I2CDisplay.c
+++ b/shared-bindings/displayio/I2CDisplay.c
@@ -131,7 +131,7 @@ STATIC mp_obj_t displayio_i2cdisplay_obj_send(mp_obj_t self, mp_obj_t command_ob
     uint8_t full_command[bufinfo.len + 1];
     full_command[0] = command;
     memcpy(full_command + 1, ((uint8_t*) bufinfo.buf), bufinfo.len);
-    common_hal_displayio_i2cdisplay_send(self, true, false, full_command, bufinfo.len + 1);
+    common_hal_displayio_i2cdisplay_send(self, DISPLAY_COMMAND, CHIP_SELECT_UNTOUCHED, full_command, bufinfo.len + 1);
     common_hal_displayio_i2cdisplay_end_transaction(self);
 
     return mp_const_none;
diff --git a/shared-bindings/displayio/I2CDisplay.h b/shared-bindings/displayio/I2CDisplay.h
index 683cff383..bae53c491 100644
--- a/shared-bindings/displayio/I2CDisplay.h
+++ b/shared-bindings/displayio/I2CDisplay.h
@@ -28,6 +28,8 @@
 #define MICROPY_INCLUDED_SHARED_BINDINGS_DISPLAYBUSIO_I2CDISPLAY_H
 
 #include "shared-module/displayio/I2CDisplay.h"
+
+#include "shared-bindings/displayio/__init__.h"
 #include "common-hal/microcontroller/Pin.h"
 
 extern const mp_obj_type_t displayio_i2cdisplay_type;
@@ -42,7 +44,7 @@ bool common_hal_displayio_i2cdisplay_bus_free(mp_obj_t self);
 
 bool common_hal_displayio_i2cdisplay_begin_transaction(mp_obj_t self);
 
-void common_hal_displayio_i2cdisplay_send(mp_obj_t self, bool command, bool toggle_every_byte, uint8_t *data, uint32_t data_length);
+void common_hal_displayio_i2cdisplay_send(mp_obj_t self, display_byte_type_t byte_type, display_chip_select_behavior_t chip_select, uint8_t *data, uint32_t data_length);
 
 void common_hal_displayio_i2cdisplay_end_transaction(mp_obj_t self);
 
diff --git a/shared-bindings/displayio/ParallelBus.c b/shared-bindings/displayio/ParallelBus.c
index a08c37679..f7195b9cc 100644
--- a/shared-bindings/displayio/ParallelBus.c
+++ b/shared-bindings/displayio/ParallelBus.c
@@ -139,8 +139,8 @@ STATIC mp_obj_t displayio_parallelbus_obj_send(mp_obj_t self, mp_obj_t command_o
     while (!common_hal_displayio_parallelbus_begin_transaction(self)) {
         RUN_BACKGROUND_TASKS;
     }
-    common_hal_displayio_parallelbus_send(self, true, false, &command, 1);
-    common_hal_displayio_parallelbus_send(self, false, false, ((uint8_t*) bufinfo.buf), bufinfo.len);
+    common_hal_displayio_parallelbus_send(self, DISPLAY_COMMAND, CHIP_SELECT_UNTOUCHED, &command, 1);
+    common_hal_displayio_parallelbus_send(self, DISPLAY_DATA, CHIP_SELECT_UNTOUCHED, ((uint8_t*) bufinfo.buf), bufinfo.len);
     common_hal_displayio_parallelbus_end_transaction(self);
 
     return mp_const_none;
diff --git a/shared-bindings/displayio/ParallelBus.h b/shared-bindings/displayio/ParallelBus.h
index a0f967332..be2aef7d4 100644
--- a/shared-bindings/displayio/ParallelBus.h
+++ b/shared-bindings/displayio/ParallelBus.h
@@ -28,8 +28,9 @@
 #define MICROPY_INCLUDED_SHARED_BINDINGS_DISPLAYBUSIO_PARALLELBUS_H
 
 #include "common-hal/displayio/ParallelBus.h"
-#include "common-hal/microcontroller/Pin.h"
 
+#include "common-hal/microcontroller/Pin.h"
+#include "shared-bindings/displayio/__init__.h"
 #include "shared-module/displayio/Group.h"
 
 extern const mp_obj_type_t displayio_parallelbus_type;
@@ -45,7 +46,7 @@ bool common_hal_displayio_parallelbus_bus_free(mp_obj_t self);
 
 bool common_hal_displayio_parallelbus_begin_transaction(mp_obj_t self);
 
-void common_hal_displayio_parallelbus_send(mp_obj_t self, bool command, bool toggle_every_byte, uint8_t *data, uint32_t data_length);
+void common_hal_displayio_parallelbus_send(mp_obj_t self, display_byte_type_t byte_type, display_chip_select_behavior_t chip_select, uint8_t *data, uint32_t data_length);
 
 void common_hal_displayio_parallelbus_end_transaction(mp_obj_t self);
 
diff --git a/shared-bindings/displayio/__init__.h b/shared-bindings/displayio/__init__.h
index 427ddb47d..122b1fbcb 100644
--- a/shared-bindings/displayio/__init__.h
+++ b/shared-bindings/displayio/__init__.h
@@ -27,6 +27,25 @@
 #ifndef MICROPY_INCLUDED_SHARED_BINDINGS_DISPLAYIO___INIT___H
 #define MICROPY_INCLUDED_SHARED_BINDINGS_DISPLAYIO___INIT___H
 
+#include "py/obj.h"
+
+typedef enum {
+    DISPLAY_COMMAND,
+    DISPLAY_DATA
+} display_byte_type_t;
+
+
+typedef enum {
+    CHIP_SELECT_UNTOUCHED,
+    CHIP_SELECT_TOGGLE_EVERY_BYTE
+} display_chip_select_behavior_t;
+
+typedef bool (*display_bus_bus_reset)(mp_obj_t bus);
+typedef bool (*display_bus_bus_free)(mp_obj_t bus);
+typedef bool (*display_bus_begin_transaction)(mp_obj_t bus);
+typedef void (*display_bus_send)(mp_obj_t bus, display_byte_type_t byte_type, display_chip_select_behavior_t chip_select, uint8_t *data, uint32_t data_length);
+typedef void (*display_bus_end_transaction)(mp_obj_t bus);
+
 void common_hal_displayio_release_displays(void);
 
 #endif  // MICROPY_INCLUDED_SHARED_BINDINGS_DISPLAYIO___INIT___H