Tweak the picodvi docs and arg checking

Author: tannewt

ports/raspberrypi/bindings/picodvi/Framebuffer.c

@@ -36,8 +36,6 @@
 #include "shared-module/framebufferio/FramebufferDisplay.h"
 
 //| class Framebuffer:
-//|     """A PicoDVI managed frame buffer."""
-//|
 //|     def __init__(
 //|         self,
 //|         width: int,
@@ -54,13 +52,14 @@
 //|         color_depth: int = 8,
 //|     ) -> None:
 //|         """Create a Framebuffer object with the given dimensions. Memory is
-//|            allocated outside of onto the heap and then moved outside on VM
-//|            end.
+//|         allocated outside of onto the heap and then moved outside on VM end.
+//|
+//|         .. warning:: This will change the system clock speed to match the DVI signal.
+//|            Make sure to initialize other objects after this one so they account
+//|            for the changed clock.
 //|
-//|         This will change the system clock speed to match the DVI signal.
-//|         Make sure to initialize other objects after this one so they account
-//|         for the changed clock. This also allocates a very large framebuffer
-//|         and is most likely to succeed the earlier it is attempted.
+//|         This allocates a very large framebuffer and is most likely to succeed
+//|         the earlier it is attempted.
 //|
 //|         Each dp and dn pair of pins must be neighboring, such as 19 and 20.
 //|         They must also be ordered the same way. In other words, dp must be
@@ -82,7 +81,7 @@
 //|         `framebufferio.FramebufferDisplay`.
 //|
 //|         :param int width: the width of the target display signal. Only 320, 400, 640 or 800 is currently supported depending on color_depth.
-//|         :param int height: the height of the target display signal. Only 240 or 480 is currently supported depenting on color_depth.
+//|         :param int height: the height of the target display signal. Only 240 or 480 is currently supported depending on color_depth.
 //|         :param ~microcontroller.Pin clk_dp: the positive clock signal pin
 //|         :param ~microcontroller.Pin clk_dn: the negative clock signal pin
 //|         :param ~microcontroller.Pin red_dp: the positive red signal pin
@@ -160,8 +159,7 @@ static void check_for_deinit(picodvi_framebuffer_obj_t *self) {
 }
 
 //|     width: int
-//|     """The width of the framebuffer, in pixels. It may be doubled for output (and half of what
-//|        width was given to __init__.)"""
+//|     """The width of the framebuffer, in pixels. It may be doubled for output."""
 STATIC mp_obj_t picodvi_framebuffer_get_width(mp_obj_t self_in) {
     picodvi_framebuffer_obj_t *self = (picodvi_framebuffer_obj_t *)self_in;
     check_for_deinit(self);
@@ -172,8 +170,7 @@ MP_PROPERTY_GETTER(picodvi_framebuffer_width_obj,
     (mp_obj_t)&picodvi_framebuffer_get_width_obj);
 
 //|     height: int
-//|     """The width of the framebuffer, in pixels. It may be doubled for output (and half of what
-//|        width was given to __init__.)"""
+//|     """The width of the framebuffer, in pixels. It may be doubled for output."""
 //|
 STATIC mp_obj_t picodvi_framebuffer_get_height(mp_obj_t self_in) {
     picodvi_framebuffer_obj_t *self = (picodvi_framebuffer_obj_t *)self_in;

ports/raspberrypi/common-hal/picodvi/Framebuffer.c

@@ -144,11 +144,11 @@ void common_hal_picodvi_framebuffer_construct(picodvi_framebuffer_obj_t *self,
 
     bool color_framebuffer = color_depth >= 8;
     const struct dvi_timing *timing = NULL;
-    if ((!color_framebuffer && width == 640 && height == 480) ||
-        (color_framebuffer && width == 320 && height == 240)) {
+    if ((width == 640 && height == 480) ||
+        (width == 320 && height == 240)) {
         timing = &dvi_timing_640x480p_60hz;
-    } else if ((!color_framebuffer && width == 800 && height == 480) ||
-               (color_framebuffer && width == 400 && height == 240)) {
+    } else if ((width == 800 && height == 480) ||
+               (width == 400 && height == 240)) {
         timing = &dvi_timing_800x480p_60hz;
     } else {
         if (height != 480 && height != 240) {
@@ -157,6 +157,12 @@ void common_hal_picodvi_framebuffer_construct(picodvi_framebuffer_obj_t *self,
         mp_raise_ValueError_varg(translate("Invalid %q"), MP_QSTR_width);
     }
 
+    // If the width is > 400, then it must not be color frame buffer and vice
+    // versa.
+    if ((width > 400) == color_framebuffer) {
+        mp_raise_ValueError_varg(translate("Invalid %q"), MP_QSTR_color_depth);
+    }
+
     bool invert_diffpairs = clk_dn->number < clk_dp->number;
     int8_t other_pins[4];
     int8_t *a;