Undo some TODO (#3024)

* TODO FL-3497: impossible to fix with current memory allocator

* TODO FL-3497: removed, requires radically different settings approach

* TODO FL-3514: removed, yes we should

* TODO FL-3498: implemented, guarding view port access with mutex.

* TODO FL-3515: removed, questionable but ok

* TODO FL-3510: removed, comment added

* TODO FL-3500: refactored, store pin numbers in GpioPinRecord, fix gpio app crash caused by incorrect gpio_pins traversal.

* Format Sources

* TODO FL-3505: removed, mutex alone is not going to fix issue with WPAN architecture

* TODO FL-3506: removed, change ownership by copy is good

* TODO FL-3519: documentation and link to source added

* Lib: remove unneded total sum from comment in bq27220

---------

Co-authored-by: hedger <hedger@users.noreply.github.com>
This commit is contained in:
あく 2023-09-01 10:54:52 +09:00 committed by GitHub
parent 7aa55ebc6c
commit f218c41d83
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 137 additions and 98 deletions

View file

@ -26,7 +26,6 @@ void test_furi_memmgr() {
mu_assert_int_eq(66, ((uint8_t*)ptr)[i]);
}
// TODO FL-3492: fix realloc to copy only old size, and write testcase that leftover of reallocated memory is zero-initialized
free(ptr);
// allocate and zero-initialize array (calloc)

View file

@ -18,10 +18,12 @@ GPIOItems* gpio_items_alloc() {
}
items->pins = malloc(sizeof(GpioPinRecord) * items->count);
for(size_t i = 0; i < items->count; i++) {
size_t index = 0;
for(size_t i = 0; i < gpio_pins_count; i++) {
if(!gpio_pins[i].debug) {
items->pins[i].pin = gpio_pins[i].pin;
items->pins[i].name = gpio_pins[i].name;
items->pins[index].pin = gpio_pins[i].pin;
items->pins[index].name = gpio_pins[i].name;
index++;
}
}
return items;

View file

@ -101,7 +101,6 @@ static void desktop_clock_draw_callback(Canvas* canvas, void* context) {
char buffer[20];
snprintf(buffer, sizeof(buffer), "%02u:%02u", hour, desktop->time_minute);
// TODO FL-3515: never do that, may cause visual glitches
view_port_set_width(
desktop->clock_viewport,
canvas_string_width(canvas, buffer) - 1 + (desktop->time_minute % 10 == 1));
@ -126,8 +125,6 @@ static bool desktop_custom_event_callback(void* context, uint32_t event) {
return true;
case DesktopGlobalAfterAppFinished:
animation_manager_load_and_continue_animation(desktop->animation_manager);
// TODO FL-3497: Implement a message mechanism for loading settings and (optionally)
// locking and unlocking
DESKTOP_SETTINGS_LOAD(&desktop->settings);
desktop_clock_reconfigure(desktop);

View file

@ -361,10 +361,11 @@ void gui_add_view_port(Gui* gui, ViewPort* view_port, GuiLayer layer) {
furi_assert(view_port);
furi_check(layer < GuiLayerMAX);
// Only fullscreen supports Vertical orientation for now
furi_assert(
ViewPortOrientation view_port_orientation = view_port_get_orientation(view_port);
furi_check(
(layer == GuiLayerFullscreen) ||
((view_port->orientation != ViewPortOrientationVertical) &&
(view_port->orientation != ViewPortOrientationVerticalFlip)));
((view_port_orientation != ViewPortOrientationVertical) &&
(view_port_orientation != ViewPortOrientationVerticalFlip)));
gui_lock(gui);
// Verify that view port is not yet added

View file

@ -272,7 +272,6 @@ void view_dispatcher_handle_input(ViewDispatcher* view_dispatcher, InputEvent* e
} else if(view_dispatcher->navigation_event_callback) {
// Dispatch navigation event
if(!view_dispatcher->navigation_event_callback(view_dispatcher->event_context)) {
// TODO FL-3514: should we allow view_dispatcher to stop without navigation_event_callback?
view_dispatcher_stop(view_dispatcher);
return;
}

View file

@ -7,8 +7,6 @@
#include "gui.h"
#include "gui_i.h"
// TODO FL-3498: add mutex to view_port ops
_Static_assert(ViewPortOrientationMAX == 4, "Incorrect ViewPortOrientation count");
_Static_assert(
(ViewPortOrientationHorizontal == 0 && ViewPortOrientationHorizontalFlip == 1 &&
@ -95,52 +93,73 @@ ViewPort* view_port_alloc() {
ViewPort* view_port = malloc(sizeof(ViewPort));
view_port->orientation = ViewPortOrientationHorizontal;
view_port->is_enabled = true;
view_port->mutex = furi_mutex_alloc(FuriMutexTypeRecursive);
return view_port;
}
void view_port_free(ViewPort* view_port) {
furi_assert(view_port);
furi_check(furi_mutex_acquire(view_port->mutex, FuriWaitForever) == FuriStatusOk);
furi_check(view_port->gui == NULL);
furi_check(furi_mutex_release(view_port->mutex) == FuriStatusOk);
furi_mutex_free(view_port->mutex);
free(view_port);
}
void view_port_set_width(ViewPort* view_port, uint8_t width) {
furi_assert(view_port);
furi_check(furi_mutex_acquire(view_port->mutex, FuriWaitForever) == FuriStatusOk);
view_port->width = width;
furi_check(furi_mutex_release(view_port->mutex) == FuriStatusOk);
}
uint8_t view_port_get_width(const ViewPort* view_port) {
furi_assert(view_port);
return view_port->width;
furi_check(furi_mutex_acquire(view_port->mutex, FuriWaitForever) == FuriStatusOk);
uint8_t width = view_port->width;
furi_check(furi_mutex_release(view_port->mutex) == FuriStatusOk);
return width;
}
void view_port_set_height(ViewPort* view_port, uint8_t height) {
furi_assert(view_port);
furi_check(furi_mutex_acquire(view_port->mutex, FuriWaitForever) == FuriStatusOk);
view_port->height = height;
furi_check(furi_mutex_release(view_port->mutex) == FuriStatusOk);
}
uint8_t view_port_get_height(const ViewPort* view_port) {
furi_assert(view_port);
return view_port->height;
furi_check(furi_mutex_acquire(view_port->mutex, FuriWaitForever) == FuriStatusOk);
uint8_t height = view_port->height;
furi_check(furi_mutex_release(view_port->mutex) == FuriStatusOk);
return height;
}
void view_port_enabled_set(ViewPort* view_port, bool enabled) {
furi_assert(view_port);
furi_check(furi_mutex_acquire(view_port->mutex, FuriWaitForever) == FuriStatusOk);
if(view_port->is_enabled != enabled) {
view_port->is_enabled = enabled;
if(view_port->gui) gui_update(view_port->gui);
}
furi_check(furi_mutex_release(view_port->mutex) == FuriStatusOk);
}
bool view_port_is_enabled(const ViewPort* view_port) {
furi_assert(view_port);
return view_port->is_enabled;
furi_check(furi_mutex_acquire(view_port->mutex, FuriWaitForever) == FuriStatusOk);
bool is_enabled = view_port->is_enabled;
furi_check(furi_mutex_release(view_port->mutex) == FuriStatusOk);
return is_enabled;
}
void view_port_draw_callback_set(ViewPort* view_port, ViewPortDrawCallback callback, void* context) {
furi_assert(view_port);
furi_check(furi_mutex_acquire(view_port->mutex, FuriWaitForever) == FuriStatusOk);
view_port->draw_callback = callback;
view_port->draw_callback_context = context;
furi_check(furi_mutex_release(view_port->mutex) == FuriStatusOk);
}
void view_port_input_callback_set(
@ -148,34 +167,43 @@ void view_port_input_callback_set(
ViewPortInputCallback callback,
void* context) {
furi_assert(view_port);
furi_check(furi_mutex_acquire(view_port->mutex, FuriWaitForever) == FuriStatusOk);
view_port->input_callback = callback;
view_port->input_callback_context = context;
furi_check(furi_mutex_release(view_port->mutex) == FuriStatusOk);
}
void view_port_update(ViewPort* view_port) {
furi_assert(view_port);
furi_check(furi_mutex_acquire(view_port->mutex, FuriWaitForever) == FuriStatusOk);
if(view_port->gui && view_port->is_enabled) gui_update(view_port->gui);
furi_check(furi_mutex_release(view_port->mutex) == FuriStatusOk);
}
void view_port_gui_set(ViewPort* view_port, Gui* gui) {
furi_assert(view_port);
furi_check(furi_mutex_acquire(view_port->mutex, FuriWaitForever) == FuriStatusOk);
view_port->gui = gui;
furi_check(furi_mutex_release(view_port->mutex) == FuriStatusOk);
}
void view_port_draw(ViewPort* view_port, Canvas* canvas) {
furi_assert(view_port);
furi_assert(canvas);
furi_check(furi_mutex_acquire(view_port->mutex, FuriWaitForever) == FuriStatusOk);
furi_check(view_port->gui);
if(view_port->draw_callback) {
view_port_setup_canvas_orientation(view_port, canvas);
view_port->draw_callback(canvas, view_port->draw_callback_context);
}
furi_check(furi_mutex_release(view_port->mutex) == FuriStatusOk);
}
void view_port_input(ViewPort* view_port, InputEvent* event) {
furi_assert(view_port);
furi_assert(event);
furi_check(furi_mutex_acquire(view_port->mutex, FuriWaitForever) == FuriStatusOk);
furi_check(view_port->gui);
if(view_port->input_callback) {
@ -183,13 +211,19 @@ void view_port_input(ViewPort* view_port, InputEvent* event) {
view_port_map_input(event, orientation);
view_port->input_callback(event, view_port->input_callback_context);
}
furi_check(furi_mutex_release(view_port->mutex) == FuriStatusOk);
}
void view_port_set_orientation(ViewPort* view_port, ViewPortOrientation orientation) {
furi_assert(view_port);
furi_check(furi_mutex_acquire(view_port->mutex, FuriWaitForever) == FuriStatusOk);
view_port->orientation = orientation;
furi_check(furi_mutex_release(view_port->mutex) == FuriStatusOk);
}
ViewPortOrientation view_port_get_orientation(const ViewPort* view_port) {
return view_port->orientation;
furi_check(furi_mutex_acquire(view_port->mutex, FuriWaitForever) == FuriStatusOk);
ViewPortOrientation orientation = view_port->orientation;
furi_check(furi_mutex_release(view_port->mutex) == FuriStatusOk);
return orientation;
}

View file

@ -10,6 +10,7 @@
struct ViewPort {
Gui* gui;
FuriMutex* mutex;
bool is_enabled;
ViewPortOrientation orientation;

View file

@ -30,7 +30,7 @@ void power_draw_battery_callback(Canvas* canvas, void* context) {
if(power->state == PowerStateCharging) {
canvas_set_bitmap_mode(canvas, 1);
canvas_set_color(canvas, ColorWhite);
// TODO FL-3510: replace -1 magic for uint8_t with re-framing
// -1 used here to overflow u8 number and render is outside of the area
canvas_draw_icon(canvas, 8, -1, &I_Charging_lightning_mask_9x10);
canvas_set_color(canvas, ColorBlack);
canvas_draw_icon(canvas, 8, -1, &I_Charging_lightning_9x10);

View file

@ -67,35 +67,53 @@ const GpioPin gpio_usb_dm = {.port = GPIOA, .pin = LL_GPIO_PIN_11};
const GpioPin gpio_usb_dp = {.port = GPIOA, .pin = LL_GPIO_PIN_12};
const GpioPinRecord gpio_pins[] = {
{.pin = &gpio_ext_pa7, .name = "PA7", .debug = false},
{.pin = &gpio_ext_pa6, .name = "PA6", .debug = false},
{.pin = &gpio_ext_pa4, .name = "PA4", .debug = false},
{.pin = &gpio_ext_pb3, .name = "PB3", .debug = false},
{.pin = &gpio_ext_pb2, .name = "PB2", .debug = false},
{.pin = &gpio_ext_pc3, .name = "PC3", .debug = false},
{.pin = &gpio_ext_pc1, .name = "PC1", .debug = false},
{.pin = &gpio_ext_pc0, .name = "PC0", .debug = false},
// 5V: 1
{.pin = &gpio_ext_pa7, .name = "PA7", .number = 2, .debug = false},
{.pin = &gpio_ext_pa6, .name = "PA6", .number = 3, .debug = false},
{.pin = &gpio_ext_pa4, .name = "PA4", .number = 4, .debug = false},
{.pin = &gpio_ext_pb3, .name = "PB3", .number = 5, .debug = false},
{.pin = &gpio_ext_pb2, .name = "PB2", .number = 6, .debug = false},
{.pin = &gpio_ext_pc3, .name = "PC3", .number = 7, .debug = false},
// GND: 8
// Space
// 3v3: 9
{.pin = &gpio_swclk, .name = "PA14", .number = 10, .debug = true},
// GND: 11
{.pin = &gpio_swdio, .name = "PA13", .number = 12, .debug = true},
{.pin = &gpio_usart_tx, .name = "PB6", .number = 13, .debug = true},
{.pin = &gpio_usart_rx, .name = "PB7", .number = 14, .debug = true},
{.pin = &gpio_ext_pc1, .name = "PC1", .number = 15, .debug = false},
{.pin = &gpio_ext_pc0, .name = "PC0", .number = 16, .debug = false},
{.pin = &gpio_ibutton, .name = "PB14", .number = 17, .debug = true},
// GND: 18
{.pin = &gpio_ext_pc5, .name = "PC5", .debug = false},
{.pin = &gpio_ext_pc4, .name = "PC4", .debug = false},
{.pin = &gpio_ext_pa5, .name = "PA5", .debug = false},
{.pin = &gpio_ext_pb9, .name = "PB9", .debug = false},
{.pin = &gpio_ext_pa0, .name = "PA0", .debug = false},
{.pin = &gpio_ext_pa1, .name = "PA1", .debug = false},
{.pin = &gpio_ext_pa15, .name = "PA15", .debug = false},
{.pin = &gpio_ext_pe4, .name = "PE4", .debug = false},
{.pin = &gpio_ext_pa2, .name = "PA2", .debug = false},
{.pin = &gpio_ext_pb4, .name = "PB4", .debug = false},
{.pin = &gpio_ext_pb5, .name = "PB5", .debug = false},
{.pin = &gpio_ext_pd0, .name = "PD0", .debug = false},
{.pin = &gpio_ext_pb13, .name = "PB13", .debug = false},
// 2nd column
// 5V: 19
{.pin = &gpio_ext_pc5, .name = "PC5", .number = 20, .debug = false},
{.pin = &gpio_ext_pc4, .name = "PC4", .number = 21, .debug = false},
{.pin = &gpio_ext_pa5, .name = "PA5", .number = 22, .debug = false},
{.pin = &gpio_ext_pb9, .name = "PB9", .number = 23, .debug = false},
{.pin = &gpio_ext_pa0, .name = "PA0", .number = 24, .debug = false},
{.pin = &gpio_ext_pa1, .name = "PA1", .number = 25, .debug = false},
// KEY: 26
// Space
// 3v3: 27
{.pin = &gpio_ext_pa15, .name = "PA15", .number = 28, .debug = false},
// GND: 29
{.pin = &gpio_ext_pe4, .name = "PE4", .number = 30, .debug = false},
{.pin = &gpio_ext_pa2, .name = "PA2", .number = 31, .debug = false},
{.pin = &gpio_ext_pb4, .name = "PB4", .number = 32, .debug = false},
{.pin = &gpio_ext_pb5, .name = "PB5", .number = 33, .debug = false},
{.pin = &gpio_ext_pd0, .name = "PD0", .number = 34, .debug = false},
{.pin = &gpio_ext_pb13, .name = "PB13", .number = 35, .debug = false},
// GND: 36
/* Dangerous pins, may damage hardware */
{.pin = &gpio_usart_rx, .name = "PB7", .debug = true},
{.pin = &gpio_speaker, .name = "PB8", .debug = true},
{.pin = &gpio_usart_rx, .name = "PB7", .number = 0, .debug = true},
{.pin = &gpio_speaker, .name = "PB8", .number = 0, .debug = true},
};
const size_t gpio_pins_count = sizeof(gpio_pins) / sizeof(GpioPinRecord);
const size_t gpio_pins_count = COUNT_OF(gpio_pins);
const InputPin input_pins[] = {
{.gpio = &gpio_button_up, .key = InputKeyUp, .inverted = true, .name = "Up"},
@ -106,7 +124,7 @@ const InputPin input_pins[] = {
{.gpio = &gpio_button_back, .key = InputKeyBack, .inverted = true, .name = "Back"},
};
const size_t input_pins_count = sizeof(input_pins) / sizeof(InputPin);
const size_t input_pins_count = COUNT_OF(input_pins);
static void furi_hal_resources_init_input_pins(GpioMode mode) {
for(size_t i = 0; i < input_pins_count; i++) {
@ -216,25 +234,10 @@ void furi_hal_resources_init() {
}
int32_t furi_hal_resources_get_ext_pin_number(const GpioPin* gpio) {
// TODO FL-3500: describe second ROW
if(gpio == &gpio_ext_pa7)
return 2;
else if(gpio == &gpio_ext_pa6)
return 3;
else if(gpio == &gpio_ext_pa4)
return 4;
else if(gpio == &gpio_ext_pb3)
return 5;
else if(gpio == &gpio_ext_pb2)
return 6;
else if(gpio == &gpio_ext_pc3)
return 7;
else if(gpio == &gpio_ext_pc1)
return 15;
else if(gpio == &gpio_ext_pc0)
return 16;
else if(gpio == &gpio_ibutton)
return 17;
else
return -1;
for(size_t i = 0; i < gpio_pins_count; i++) {
if(gpio_pins[i].pin == gpio) {
return gpio_pins[i].number;
}
}
return -1;
}

View file

@ -41,6 +41,7 @@ typedef struct {
typedef struct {
const GpioPin* pin;
const char* name;
const uint8_t number;
const bool debug;
} GpioPinRecord;

View file

@ -222,7 +222,6 @@ bool ble_glue_wait_for_c2_start(int32_t timeout) {
bool started = false;
do {
// TODO FL-3505: use mutex?
started = ble_glue->status == BleGlueStatusC2Started;
if(!started) {
timeout--;

View file

@ -14,7 +14,6 @@ void flipper_gatt_characteristic_init(
furi_assert(char_instance);
// Copy the descriptor to the instance, since it may point to stack memory
// TODO FL-3506: only copy if really comes from stack
char_instance->characteristic = malloc(sizeof(FlipperGattCharacteristicParams));
memcpy(
(void*)char_instance->characteristic,

View file

@ -69,22 +69,32 @@ const GpioPin gpio_usb_dm = {.port = GPIOA, .pin = LL_GPIO_PIN_11};
const GpioPin gpio_usb_dp = {.port = GPIOA, .pin = LL_GPIO_PIN_12};
const GpioPinRecord gpio_pins[] = {
{.pin = &gpio_ext_pa7, .name = "PA7", .debug = false},
{.pin = &gpio_ext_pa6, .name = "PA6", .debug = false},
{.pin = &gpio_ext_pa4, .name = "PA4", .debug = false},
{.pin = &gpio_ext_pb3, .name = "PB3", .debug = false},
{.pin = &gpio_ext_pb2, .name = "PB2", .debug = false},
{.pin = &gpio_ext_pc3, .name = "PC3", .debug = false},
{.pin = &gpio_ext_pc1, .name = "PC1", .debug = false},
{.pin = &gpio_ext_pc0, .name = "PC0", .debug = false},
// 5V: 1
{.pin = &gpio_ext_pa7, .name = "PA7", .number = 2, .debug = false},
{.pin = &gpio_ext_pa6, .name = "PA6", .number = 3, .debug = false},
{.pin = &gpio_ext_pa4, .name = "PA4", .number = 4, .debug = false},
{.pin = &gpio_ext_pb3, .name = "PB3", .number = 5, .debug = false},
{.pin = &gpio_ext_pb2, .name = "PB2", .number = 6, .debug = false},
{.pin = &gpio_ext_pc3, .name = "PC3", .number = 7, .debug = false},
// GND: 8
// Space
// 3v3: 9
{.pin = &gpio_swclk, .name = "PA14", .number = 10, .debug = true},
// GND: 11
{.pin = &gpio_swdio, .name = "PA13", .number = 12, .debug = true},
{.pin = &gpio_usart_tx, .name = "PB6", .number = 13, .debug = true},
{.pin = &gpio_usart_rx, .name = "PB7", .number = 14, .debug = true},
{.pin = &gpio_ext_pc1, .name = "PC1", .number = 15, .debug = false},
{.pin = &gpio_ext_pc0, .name = "PC0", .number = 16, .debug = false},
{.pin = &gpio_ibutton, .name = "PB14", .number = 17, .debug = true},
// GND: 18
/* Dangerous pins, may damage hardware */
{.pin = &gpio_usart_rx, .name = "PB7", .debug = true},
{.pin = &gpio_speaker, .name = "PB8", .debug = true},
{.pin = &gpio_infrared_tx, .name = "PB9", .debug = true},
};
const size_t gpio_pins_count = sizeof(gpio_pins) / sizeof(GpioPinRecord);
const size_t gpio_pins_count = COUNT_OF(gpio_pins);
const InputPin input_pins[] = {
{.gpio = &gpio_button_up, .key = InputKeyUp, .inverted = true, .name = "Up"},
@ -95,7 +105,7 @@ const InputPin input_pins[] = {
{.gpio = &gpio_button_back, .key = InputKeyBack, .inverted = true, .name = "Back"},
};
const size_t input_pins_count = sizeof(input_pins) / sizeof(InputPin);
const size_t input_pins_count = COUNT_OF(input_pins);
static void furi_hal_resources_init_input_pins(GpioMode mode) {
for(size_t i = 0; i < input_pins_count; i++) {
@ -210,24 +220,10 @@ void furi_hal_resources_init() {
}
int32_t furi_hal_resources_get_ext_pin_number(const GpioPin* gpio) {
if(gpio == &gpio_ext_pa7)
return 2;
else if(gpio == &gpio_ext_pa6)
return 3;
else if(gpio == &gpio_ext_pa4)
return 4;
else if(gpio == &gpio_ext_pb3)
return 5;
else if(gpio == &gpio_ext_pb2)
return 6;
else if(gpio == &gpio_ext_pc3)
return 7;
else if(gpio == &gpio_ext_pc1)
return 15;
else if(gpio == &gpio_ext_pc0)
return 16;
else if(gpio == &gpio_ibutton)
return 17;
else
return -1;
for(size_t i = 0; i < gpio_pins_count; i++) {
if(gpio_pins[i].pin == gpio) {
return gpio_pins[i].number;
}
}
return -1;
}

View file

@ -41,6 +41,7 @@ typedef struct {
typedef struct {
const GpioPin* pin;
const char* name;
const uint8_t number;
const bool debug;
} GpioPinRecord;

View file

@ -54,6 +54,10 @@ static bool bq27220_parameter_check(
}
if(update) {
// Datasheet contains incorrect procedure for memory update, more info:
// https://e2e.ti.com/support/power-management-group/power-management/f/power-management-forum/719878/bq27220-technical-reference-manual-sluubd4-is-missing-extended-data-commands-chapter
// 2. Write the address AND the parameter data to 0x3E+ (auto increment)
if(!furi_hal_i2c_write_mem(
handle,
BQ27220_ADDRESS,
@ -67,9 +71,12 @@ static bool bq27220_parameter_check(
furi_delay_us(10000);
// 3. Calculate the check sum: 0xFF - (sum of address and data) OR 0xFF
uint8_t checksum = bq27220_get_checksum(buffer, size + 2);
// 4. Write the check sum to 0x60 and the total length of (address + parameter data + check sum + length) to 0x61
buffer[0] = checksum;
buffer[1] = 4 + size; // TODO FL-3519: why 4?
// 2 bytes address, `size` bytes data, 1 byte check sum, 1 byte length
buffer[1] = 2 + size + 1 + 1;
if(!furi_hal_i2c_write_mem(
handle, BQ27220_ADDRESS, CommandMACDataSum, buffer, 2, BQ27220_I2C_TIMEOUT)) {
FURI_LOG_I(TAG, "CRC write failed");