Skip to content

Commit 786f4ed

Browse files
tannewttannewt
authored
Merge pull request adafruit#3344 from jepler/issue-3184
Fix RGBMatrix, FrameBufferDisplay bugs
2 parents f27b896 + 17a5a85 commit 786f4ed

9 files changed

Lines changed: 144 additions & 73 deletions

File tree

‎lib/protomatter‎

Submodule protomatter updated 1 file

‎ports/atmel-samd/Makefile‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ CFLAGS += -DCFG_TUSB_MCU=OPT_MCU_SAME5X -DCFG_TUD_MIDI_RX_BUFSIZE=128 -DCFG_TUD_
107107
endif
108108

109109
# option to override default optimization level, set in boards/$(BOARD)/mpconfigboard.mk
110-
CFLAGS += $(OPTIMIZATION_FLAGS) -DNDEBUG
110+
CFLAGS += $(OPTIMIZATION_FLAGS)
111111

112112
$(echo PERIPHERALS_CHIP_FAMILY=$(PERIPHERALS_CHIP_FAMILY))
113113
#Debugging/Optimization
@@ -121,6 +121,7 @@ ifeq ($(DEBUG), 1)
121121
CFLAGS += -DENABLE_MICRO_TRACE_BUFFER
122122
endif
123123
else
124+
CFLAGS += -DNDEBUG
124125
# -finline-limit can shrink the image size.
125126
# -finline-limit=80 or so is similar to not having it on.
126127
# There is no simple default value, though.
@@ -257,6 +258,8 @@ SRC_ASF += \
257258
$(BUILD)/asf4/$(CHIP_FAMILY)/hpl/sdhc/hpl_sdhc.o: CFLAGS += -Wno-cast-align
258259
endif
259260

261+
$(BUILD)/asf4/$(CHIP_FAMILY)/hpl/sercom/hpl_sercom.o: CFLAGS += -Wno-maybe-uninitialized
262+
260263
SRC_ASF := $(addprefix asf4/$(CHIP_FAMILY)/, $(SRC_ASF))
261264

262265
SRC_C += \

‎shared-bindings/rgbmatrix/RGBMatrix.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ STATIC mp_obj_t rgbmatrix_rgbmatrix_deinit(mp_obj_t self_in) {
252252
STATIC MP_DEFINE_CONST_FUN_OBJ_1(rgbmatrix_rgbmatrix_deinit_obj, rgbmatrix_rgbmatrix_deinit);
253253

254254
static void check_for_deinit(rgbmatrix_rgbmatrix_obj_t *self) {
255-
if (!self->core.rgbPins) {
255+
if (!self->protomatter.rgbPins) {
256256
raise_deinited_error();
257257
}
258258
}

‎shared-bindings/rgbmatrix/RGBMatrix.h‎

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -24,29 +24,12 @@
2424
* THE SOFTWARE.
2525
*/
2626

27-
#ifndef MICROPY_INCLUDED_SHARED_BINDINGS_RGBMATRIX_RGBMATRIX_H
28-
#define MICROPY_INCLUDED_SHARED_BINDINGS_RGBMATRIX_RGBMATRIX_H
27+
#pragma once
2928

3029
#include "shared-module/rgbmatrix/RGBMatrix.h"
3130
#include "lib/protomatter/core.h"
3231

3332
extern const mp_obj_type_t rgbmatrix_RGBMatrix_type;
34-
typedef struct {
35-
mp_obj_base_t base;
36-
mp_obj_t framebuffer;
37-
mp_buffer_info_t bufinfo;
38-
Protomatter_core core;
39-
void *timer;
40-
uint16_t bufsize, width;
41-
uint8_t rgb_pins[30];
42-
uint8_t addr_pins[10];
43-
uint8_t clock_pin, latch_pin, oe_pin;
44-
uint8_t rgb_count, addr_count;
45-
uint8_t bit_depth;
46-
bool core_is_initialized;
47-
bool paused;
48-
bool doublebuffer;
49-
} rgbmatrix_rgbmatrix_obj_t;
5033

5134
void common_hal_rgbmatrix_rgbmatrix_construct(rgbmatrix_rgbmatrix_obj_t* self, int width, int bit_depth, uint8_t rgb_count, uint8_t* rgb_pins, uint8_t addr_count, uint8_t* addr_pins, uint8_t clock_pin, uint8_t latch_pin, uint8_t oe_pin, bool doublebuffer, mp_obj_t framebuffer, void* timer);
5235
void common_hal_rgbmatrix_rgbmatrix_deinit(rgbmatrix_rgbmatrix_obj_t*);
@@ -57,5 +40,3 @@ bool common_hal_rgbmatrix_rgbmatrix_get_paused(rgbmatrix_rgbmatrix_obj_t* self);
5740
void common_hal_rgbmatrix_rgbmatrix_refresh(rgbmatrix_rgbmatrix_obj_t* self);
5841
int common_hal_rgbmatrix_rgbmatrix_get_width(rgbmatrix_rgbmatrix_obj_t* self);
5942
int common_hal_rgbmatrix_rgbmatrix_get_height(rgbmatrix_rgbmatrix_obj_t* self);
60-
61-
#endif

‎shared-module/framebufferio/FramebufferDisplay.c‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ void common_hal_framebufferio_framebufferdisplay_construct(framebufferio_framebu
6666
ram_height,
6767
0,
6868
0,
69-
rotation,
69+
0, // rotation
7070
depth,
7171
fb_getter_default(get_grayscale, (depth < 8)),
7272
fb_getter_default(get_pixels_in_byte_share_row, false),
@@ -92,6 +92,10 @@ void common_hal_framebufferio_framebufferdisplay_construct(framebufferio_framebu
9292
self->native_frames_per_second = fb_getter_default(get_native_frames_per_second, 60);
9393
self->native_ms_per_frame = 1000 / self->native_frames_per_second;
9494

95+
if (rotation != 0) {
96+
common_hal_framebufferio_framebufferdisplay_set_rotation(self, rotation);
97+
}
98+
9599
supervisor_start_terminal(self->core.width, self->core.height);
96100

97101
// Set the group after initialization otherwise we may send pixels while we delay in

‎shared-module/rgbmatrix/RGBMatrix.c‎

Lines changed: 47 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,9 @@ void common_hal_rgbmatrix_rgbmatrix_construct(rgbmatrix_rgbmatrix_obj_t *self, i
6666
}
6767

6868
void common_hal_rgbmatrix_rgbmatrix_reconstruct(rgbmatrix_rgbmatrix_obj_t* self, mp_obj_t framebuffer) {
69+
common_hal_rgbmatrix_timer_disable(self->timer);
6970
if (framebuffer) {
7071
self->framebuffer = framebuffer;
71-
framebuffer = mp_obj_new_bytearray_of_zeros(self->bufsize);
7272
mp_get_buffer_raise(self->framebuffer, &self->bufinfo, MP_BUFFER_READ);
7373
if (mp_get_buffer(self->framebuffer, &self->bufinfo, MP_BUFFER_RW)) {
7474
self->bufinfo.typecode = 'H' | MP_OBJ_ARRAY_TYPECODE_FLAG_RW;
@@ -79,36 +79,40 @@ void common_hal_rgbmatrix_rgbmatrix_reconstruct(rgbmatrix_rgbmatrix_obj_t* self,
7979
mp_get_index(mp_obj_get_type(self->framebuffer), self->bufinfo.len, MP_OBJ_NEW_SMALL_INT(self->bufsize-1), false);
8080
} else {
8181
_PM_FREE(self->bufinfo.buf);
82-
_PM_FREE(self->core.rgbPins);
83-
_PM_FREE(self->core.addr);
84-
_PM_FREE(self->core.screenData);
82+
_PM_FREE(self->protomatter.rgbPins);
83+
_PM_FREE(self->protomatter.addr);
84+
_PM_FREE(self->protomatter.screenData);
8585

8686
self->framebuffer = NULL;
87-
self->bufinfo.buf = _PM_allocator_impl(self->bufsize);
87+
self->bufinfo.buf = common_hal_rgbmatrix_allocator_impl(self->bufsize);
8888
self->bufinfo.len = self->bufsize;
8989
self->bufinfo.typecode = 'H' | MP_OBJ_ARRAY_TYPECODE_FLAG_RW;
9090
}
9191

92-
ProtomatterStatus stat = _PM_init(&self->core,
92+
memset(&self->protomatter, 0, sizeof(self->protomatter));
93+
ProtomatterStatus stat = _PM_init(&self->protomatter,
9394
self->width, self->bit_depth,
9495
self->rgb_count/6, self->rgb_pins,
9596
self->addr_count, self->addr_pins,
9697
self->clock_pin, self->latch_pin, self->oe_pin,
9798
self->doublebuffer, self->timer);
9899

99100
if (stat == PROTOMATTER_OK) {
100-
_PM_protoPtr = &self->core;
101+
_PM_protoPtr = &self->protomatter;
101102
common_hal_mcu_disable_interrupts();
102103
common_hal_rgbmatrix_timer_enable(self->timer);
103-
stat = _PM_begin(&self->core);
104-
_PM_convert_565(&self->core, self->bufinfo.buf, self->width);
104+
stat = _PM_begin(&self->protomatter);
105+
106+
if (stat == PROTOMATTER_OK) {
107+
_PM_convert_565(&self->protomatter, self->bufinfo.buf, self->width);
108+
}
105109
common_hal_mcu_enable_interrupts();
106-
_PM_swapbuffer_maybe(&self->core);
110+
if (stat == PROTOMATTER_OK) {
111+
_PM_swapbuffer_maybe(&self->protomatter);
112+
}
107113
}
108114

109115
if (stat != PROTOMATTER_OK) {
110-
// XXX this deinit() actually makes crashy-crashy
111-
// can trigger it by sending inappropriate pins
112116
common_hal_rgbmatrix_rgbmatrix_deinit(self);
113117
switch (stat) {
114118
case PROTOMATTER_ERR_PINS:
@@ -117,7 +121,9 @@ void common_hal_rgbmatrix_rgbmatrix_reconstruct(rgbmatrix_rgbmatrix_obj_t* self,
117121
case PROTOMATTER_ERR_ARG:
118122
mp_raise_ValueError(translate("Invalid argument"));
119123
break;
120-
case PROTOMATTER_ERR_MALLOC: /// should have already been signaled as NLR
124+
case PROTOMATTER_ERR_MALLOC:
125+
mp_raise_msg(&mp_type_MemoryError, NULL);
126+
break;
121127
default:
122128
mp_raise_msg_varg(&mp_type_RuntimeError,
123129
translate("Internal error #%d"), (int)stat);
@@ -126,7 +132,6 @@ void common_hal_rgbmatrix_rgbmatrix_reconstruct(rgbmatrix_rgbmatrix_obj_t* self,
126132
}
127133

128134
self->paused = 0;
129-
130135
}
131136

132137
STATIC void free_pin(uint8_t *pin) {
@@ -148,7 +153,7 @@ void common_hal_rgbmatrix_rgbmatrix_deinit(rgbmatrix_rgbmatrix_obj_t* self) {
148153
self->timer = 0;
149154
}
150155

151-
if (_PM_protoPtr == &self->core) {
156+
if (_PM_protoPtr == &self->protomatter) {
152157
_PM_protoPtr = NULL;
153158
}
154159

@@ -158,10 +163,10 @@ void common_hal_rgbmatrix_rgbmatrix_deinit(rgbmatrix_rgbmatrix_obj_t* self) {
158163
free_pin(&self->latch_pin);
159164
free_pin(&self->oe_pin);
160165

161-
if (self->core.rgbPins) {
162-
_PM_free(&self->core);
166+
if (self->protomatter.rgbPins) {
167+
_PM_free(&self->protomatter);
163168
}
164-
memset(&self->core, 0, sizeof(self->core));
169+
memset(&self->protomatter, 0, sizeof(self->protomatter));
165170

166171
// If it was supervisor-allocated, it is supervisor-freed and the pointer
167172
// is zeroed, otherwise the pointer is just zeroed
@@ -175,16 +180,16 @@ void common_hal_rgbmatrix_rgbmatrix_deinit(rgbmatrix_rgbmatrix_obj_t* self) {
175180

176181
void rgbmatrix_rgbmatrix_collect_ptrs(rgbmatrix_rgbmatrix_obj_t* self) {
177182
gc_collect_ptr(self->framebuffer);
178-
gc_collect_ptr(self->core.rgbPins);
179-
gc_collect_ptr(self->core.addr);
180-
gc_collect_ptr(self->core.screenData);
183+
gc_collect_ptr(self->protomatter.rgbPins);
184+
gc_collect_ptr(self->protomatter.addr);
185+
gc_collect_ptr(self->protomatter.screenData);
181186
}
182187

183188
void common_hal_rgbmatrix_rgbmatrix_set_paused(rgbmatrix_rgbmatrix_obj_t* self, bool paused) {
184189
if (paused && !self->paused) {
185-
_PM_stop(&self->core);
190+
_PM_stop(&self->protomatter);
186191
} else if (!paused && self->paused) {
187-
_PM_resume(&self->core);
192+
_PM_resume(&self->protomatter);
188193
}
189194
self->paused = paused;
190195
}
@@ -194,8 +199,8 @@ bool common_hal_rgbmatrix_rgbmatrix_get_paused(rgbmatrix_rgbmatrix_obj_t* self)
194199
}
195200

196201
void common_hal_rgbmatrix_rgbmatrix_refresh(rgbmatrix_rgbmatrix_obj_t* self) {
197-
_PM_convert_565(&self->core, self->bufinfo.buf, self->width);
198-
_PM_swapbuffer_maybe(&self->core);
202+
_PM_convert_565(&self->protomatter, self->bufinfo.buf, self->width);
203+
_PM_swapbuffer_maybe(&self->protomatter);
199204
}
200205

201206
int common_hal_rgbmatrix_rgbmatrix_get_width(rgbmatrix_rgbmatrix_obj_t* self) {
@@ -206,3 +211,20 @@ int common_hal_rgbmatrix_rgbmatrix_get_height(rgbmatrix_rgbmatrix_obj_t* self) {
206211
int computed_height = (self->rgb_count / 3) << (self->addr_count);
207212
return computed_height;
208213
}
214+
215+
void *common_hal_rgbmatrix_allocator_impl(size_t sz) {
216+
if (gc_alloc_possible()) {
217+
return m_malloc_maybe(sz + sizeof(void*), true);
218+
} else {
219+
supervisor_allocation *allocation = allocate_memory(align32_size(sz), false);
220+
return allocation ? allocation->ptr : NULL;
221+
}
222+
}
223+
224+
void common_hal_rgbmatrix_free_impl(void *ptr_in) {
225+
supervisor_allocation *allocation = allocation_from_ptr(ptr_in);
226+
227+
if (allocation) {
228+
free_memory(allocation);
229+
}
230+
}

‎shared-module/rgbmatrix/RGBMatrix.h‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/*
2+
* This file is part of the Micro Python project, http://micropython.org/
3+
*
4+
* The MIT License (MIT)
5+
*
6+
* Copyright (c) 2020 Jeff Epler for Adafruit Industries
7+
*
8+
* Permission is hereby granted, free of charge, to any person obtaining a copy
9+
* of this software and associated documentation files (the "Software"), to deal
10+
* in the Software without restriction, including without limitation the rights
11+
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
12+
* copies of the Software, and to permit persons to whom the Software is
13+
* furnished to do so, subject to the following conditions:
14+
*
15+
* The above copyright notice and this permission notice shall be included in
16+
* all copies or substantial portions of the Software.
17+
*
18+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
19+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
20+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
21+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
22+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
23+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
24+
* THE SOFTWARE.
25+
*/
26+
27+
#pragma once
28+
29+
#include "lib/protomatter/core.h"
30+
31+
extern const mp_obj_type_t rgbmatrix_RGBMatrix_type;
32+
typedef struct {
33+
mp_obj_base_t base;
34+
mp_obj_t framebuffer;
35+
mp_buffer_info_t bufinfo;
36+
Protomatter_core protomatter;
37+
void *timer;
38+
uint16_t bufsize, width;
39+
uint8_t rgb_pins[30];
40+
uint8_t addr_pins[10];
41+
uint8_t clock_pin, latch_pin, oe_pin;
42+
uint8_t rgb_count, addr_count;
43+
uint8_t bit_depth;
44+
bool core_is_initialized;
45+
bool paused;
46+
bool doublebuffer;
47+
} rgbmatrix_rgbmatrix_obj_t;

‎shared-module/rgbmatrix/allocator.h‎

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,37 @@
1-
#ifndef MICROPY_INCLUDED_SHARED_MODULE_RGBMATRIX_ALLOCATOR_H
2-
#define MICROPY_INCLUDED_SHARED_MODULE_RGBMATRIX_ALLOCATOR_H
1+
/*
2+
* This file is part of the Micro Python project, http://micropython.org/
3+
*
4+
* The MIT License (MIT)
5+
*
6+
* Copyright (c) 2020 Jeff Epler for Adafruit Industries
7+
*
8+
* Permission is hereby granted, free of charge, to any person obtaining a copy
9+
* of this software and associated documentation files (the "Software"), to deal
10+
* in the Software without restriction, including without limitation the rights
11+
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
12+
* copies of the Software, and to permit persons to whom the Software is
13+
* furnished to do so, subject to the following conditions:
14+
*
15+
* The above copyright notice and this permission notice shall be included in
16+
* all copies or substantial portions of the Software.
17+
*
18+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
19+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
20+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
21+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
22+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
23+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
24+
* THE SOFTWARE.
25+
*/
26+
27+
#pragma once
328

429
#include <stdbool.h>
530
#include "py/gc.h"
631
#include "py/misc.h"
732
#include "supervisor/memory.h"
833

9-
#define _PM_ALLOCATOR _PM_allocator_impl
10-
#define _PM_FREE(x) (_PM_free_impl((x)), (x)=NULL, (void)0)
11-
12-
static inline void *_PM_allocator_impl(size_t sz) {
13-
if (gc_alloc_possible()) {
14-
return m_malloc(sz + sizeof(void*), true);
15-
} else {
16-
supervisor_allocation *allocation = allocate_memory(align32_size(sz), false);
17-
return allocation ? allocation->ptr : NULL;
18-
}
19-
}
20-
21-
static inline void _PM_free_impl(void *ptr_in) {
22-
supervisor_allocation *allocation = allocation_from_ptr(ptr_in);
23-
24-
if (allocation) {
25-
free_memory(allocation);
26-
}
27-
}
28-
29-
#endif
34+
#define _PM_ALLOCATOR common_hal_rgbmatrix_allocator_impl
35+
#define _PM_FREE(x) (common_hal_rgbmatrix_free_impl((x)), (x)=NULL, (void)0)
36+
extern void *common_hal_rgbmatrix_allocator_impl(size_t sz);
37+
extern void common_hal_rgbmatrix_free_impl(void *);

‎supervisor/shared/display.c‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,14 @@ void supervisor_start_terminal(uint16_t width_px, uint16_t height_px) {
6565
if (width_in_tiles < 80) {
6666
scale = 1;
6767
}
68+
6869
width_in_tiles = (width_px - blinka_bitmap.width * scale) / (grid->tile_width * scale);
70+
if (width_in_tiles < 1) {
71+
width_in_tiles = 1;
72+
}
6973
uint16_t height_in_tiles = height_px / (grid->tile_height * scale);
7074
uint16_t remaining_pixels = height_px % (grid->tile_height * scale);
71-
if (remaining_pixels > 0) {
75+
if (height_in_tiles < 1 || remaining_pixels > 0) {
7276
height_in_tiles += 1;
7377
}
7478

@@ -94,6 +98,8 @@ void supervisor_start_terminal(uint16_t width_px, uint16_t height_px) {
9498
}
9599
grid->width_in_tiles = width_in_tiles;
96100
grid->height_in_tiles = height_in_tiles;
101+
assert(width_in_tiles > 0);
102+
assert(height_in_tiles > 0);
97103
grid->pixel_width = width_in_tiles * grid->tile_width;
98104
grid->pixel_height = height_in_tiles * grid->tile_height;
99105
grid->tiles = tiles;

0 commit comments

Comments
 (0)