Hackathone session: bugfixes and documentation update (#869)

* ReadMe: update flashing scripts section
* Furi: add record exists method to record store.
* FuriHal: early OS init and i2c timeouts based on os ticks.
* Storage: replace malloc with furi_alloc, fix errors found by pvs.
* iButton: properly handle shutdown in cli search command
* SubGhz: proper argument type in sscanf and incorrect position of logging in switch.

ReadMe.md

@@ -14,24 +14,14 @@ Our goal is to create nice and clean code with good documentation, to make it a
 
 Flipper Zero's firmware consists of three components:
 
-- Core2 firmware set - proprietary components by ST: FUS + radio stack.
+- Core2 firmware set - proprietary components by ST: FUS + radio stack. FUS is flashed at factory and you should never update it.
-- Core1 Bootloader - controls basic hardware initialization and loads firmware
+- Core1 Bootloader - controls basic hardware initialization and loads firmware.
-- Core1 Firmware - HAL + OS + Drivers + Applications
+- Core1 Firmware - HAL + OS + Drivers + Applications.
 
 All 3 of them must be flashed in order described.
 
 ## With STLink
 
-### Core2 flashing procedures
-
-Prerequisites:
-
-- Linux / macOS
-- Terminal
-- STM32_Programmer_CLI added to $PATH
-
-One liner: `./flash_core2_ble.sh`
-
 ### Core1 Bootloader + Firmware
 
 Prerequisites:
@@ -41,7 +31,17 @@ Prerequisites:
 - [arm-gcc-none-eabi](https://developer.arm.com/tools-and-software/open-source-software/developer-tools/gnu-toolchain/gnu-rm/downloads)
 - openocd
 
-One liner: `./flash_core1_main.sh`
+One liner: `make flash`
+
+### Core2 flashing procedures
+
+Prerequisites:
+
+- Linux / macOS
+- Terminal
+- STM32_Programmer_CLI (v2.5.0) added to $PATH
+
+One liner: `make flash_radio`
 
 ## With USB DFU 
 
@@ -61,6 +61,7 @@ One liner: `./flash_core1_main.sh`
 
 1. Install [Docker Engine and Docker Compose](https://www.docker.com/get-started)
 2. Prepare the container:
+
  ```sh
  docker-compose up -d
  ```

applications/ibutton/ibutton-cli.cpp

@@ -255,7 +255,6 @@ void onewire_cli_search(Cli* cli) {
             printf("Search finished\r\n");
             onewire.reset_search();
             done = true;
-            return;
         } else {
             printf("Found: ");
             for(uint8_t i = 0; i < 8; i++) {

applications/storage/storage-processing.c

@@ -377,7 +377,7 @@ static FS_Error storage_process_common_rename(Storage* app, const char* old, con
     StorageType type_old = storage_get_type_by_path(old);
     StorageType type_new = storage_get_type_by_path(new);
 
-    if(storage_type_is_not_valid(type_old) || storage_type_is_not_valid(type_old)) {
+    if(storage_type_is_not_valid(type_old) || storage_type_is_not_valid(type_new)) {
         ret = FSE_INVALID_NAME;
     } else {
         if(type_old != type_new) {

applications/storage/storages/storage-ext.c

@@ -512,7 +512,7 @@ static FS_Error storage_ext_common_fs_info(
 /******************* Init Storage *******************/
 
 void storage_ext_init(StorageData* storage) {
-    SDData* sd_data = malloc(sizeof(SDData));
+    SDData* sd_data = furi_alloc(sizeof(SDData));
     sd_data->fs = &USERFatFS;
     sd_data->path = "0:/";
     sd_data->sd_was_present = true;

applications/subghz/subghz_cli.c

@@ -102,13 +102,13 @@ static void subghz_cli_command_rx_carrier(Cli* cli, string_t args, void* context
 static void subghz_cli_command_tx(Cli* cli, string_t args, void* context) {
     uint32_t frequency = 433920000;
     uint32_t key = 0x0074BADE;
-    size_t repeat = 10;
+    uint32_t repeat = 10;
 
     if(string_size(args)) {
-        int ret = sscanf(string_get_cstr(args), "%lx %lu %u", &key, &frequency, &repeat);
+        int ret = sscanf(string_get_cstr(args), "%lx %lu %lu", &key, &frequency, &repeat);
         if(ret != 3) {
             printf(
-                "sscanf returned %d, key: %lx, frequency: %lu, repeat: %u\r\n",
+                "sscanf returned %d, key: %lx, frequency: %lu, repeat: %lu\r\n",
                 ret,
                 key,
                 frequency,
@@ -128,7 +128,7 @@ static void subghz_cli_command_tx(Cli* cli, string_t args, void* context) {
     }
 
     printf(
-        "Transmitting at %lu, key %lx, repeat %u. Press CTRL+C to stop\r\n",
+        "Transmitting at %lu, key %lx, repeat %lu. Press CTRL+C to stop\r\n",
         frequency,
         key,
         repeat);

applications/subghz/subghz_i.c

@@ -41,8 +41,8 @@ bool subghz_get_preset_name(SubGhz* subghz, string_t preset) {
     case FuriHalSubGhzPreset2FSKDev476Async:
         preset_name = "FuriHalSubGhzPreset2FSKDev476Async";
         break;
-        FURI_LOG_E(SUBGHZ_PARSER_TAG, "Unknown preset");
     default:
+        FURI_LOG_E(SUBGHZ_PARSER_TAG, "Unknown preset");
         return false;
         break;
     }

core/furi/record.c

@@ -52,6 +52,24 @@ static void furi_record_unlock() {
     furi_check(osMutexRelease(furi_record->mutex) == osOK);
 }
 
+bool furi_record_exists(const char* name) {
+    furi_assert(furi_record);
+    furi_assert(name);
+
+    bool ret = false;
+
+    string_t name_str;
+    string_init_set_str(name_str, name);
+
+    furi_record_lock();
+    ret = (FuriRecordDataDict_get(furi_record->records, name_str) != NULL);
+    furi_record_unlock();
+
+    string_clear(name_str);
+
+    return ret;
+}
+
 void furi_record_create(const char* name, void* data) {
     furi_assert(furi_record);
 

core/furi/record.h

@@ -15,6 +15,14 @@ extern "C" {
  */
 void furi_record_init();
 
+/** Check if record exists
+ *
+ * @param      name  record name
+ * @note       Thread safe. Create and destroy must be executed from the same
+ *             thread.
+ */
+bool furi_record_exists(const char* name);
+
 /** Create record
  *
  * @param      name  record name

firmware/targets/f6/fatfs/stm32_adafruit_sd.c

@@ -405,7 +405,7 @@ BSP_SD_ReadBlocks(uint32_t* pData, uint32_t ReadAddr, uint32_t NumOfBlocks, uint
         goto error;
     }
 
-    ptr = malloc(sizeof(uint8_t) * BlockSize);
+    ptr = furi_alloc(sizeof(uint8_t) * BlockSize);
     if(ptr == NULL) {
         goto error;
     }
@@ -483,7 +483,7 @@ BSP_SD_WriteBlocks(uint32_t* pData, uint32_t WriteAddr, uint32_t NumOfBlocks, ui
         goto error;
     }
 
-    ptr = malloc(sizeof(uint8_t) * BlockSize);
+    ptr = furi_alloc(sizeof(uint8_t) * BlockSize);
     if(ptr == NULL) {
         goto error;
     }

firmware/targets/f6/furi-hal/furi-hal-i2c.c

@@ -46,12 +46,24 @@ bool furi_hal_i2c_tx(
     const uint8_t* data,
     uint8_t size,
     uint32_t timeout) {
-    furi_check(handle->bus->current_handle == handle);
-    uint32_t time_left = timeout;
-    bool ret = true;
 
-    while(LL_I2C_IsActiveFlag_BUSY(handle->bus->i2c))
+    furi_check(handle->bus->current_handle == handle);
-        ;
+    furi_assert(timeout > 0);
+
+    bool ret = true;
+    uint32_t timeout_tick = osKernelGetTickCount() + timeout;
+
+    do {
+        while(LL_I2C_IsActiveFlag_BUSY(handle->bus->i2c)) {
+            if(osKernelGetTickCount() >= timeout_tick) {
+                ret = false;
+                break;
+            }
+        }
+
+        if(!ret) {
+            break;
+        }
 
         LL_I2C_HandleTransfer(
             handle->bus->i2c,
@@ -66,18 +78,16 @@ bool furi_hal_i2c_tx(
                 LL_I2C_TransmitData8(handle->bus->i2c, (*data));
                 data++;
                 size--;
-            time_left = timeout;
             }
 
-        if(LL_SYSTICK_IsActiveCounterFlag()) {
+            if(osKernelGetTickCount() >= timeout_tick) {
-            if(--time_left == 0) {
                 ret = false;
                 break;
             }
         }
-    }
 
         LL_I2C_ClearFlag_STOP(handle->bus->i2c);
+    } while(0);
 
     return ret;
 }
@@ -88,12 +98,24 @@ bool furi_hal_i2c_rx(
     uint8_t* data,
     uint8_t size,
     uint32_t timeout) {
-    furi_check(handle->bus->current_handle == handle);
-    uint32_t time_left = timeout;
-    bool ret = true;
 
-    while(LL_I2C_IsActiveFlag_BUSY(handle->bus->i2c))
+    furi_check(handle->bus->current_handle == handle);
-        ;
+    furi_assert(timeout > 0);
+
+    bool ret = true;
+    uint32_t timeout_tick = osKernelGetTickCount() + timeout;
+
+    do {
+        while(LL_I2C_IsActiveFlag_BUSY(handle->bus->i2c)) {
+            if(osKernelGetTickCount() >= timeout_tick) {
+                ret = false;
+                break;
+            }
+        }
+
+        if(!ret) {
+            break;
+        }
 
         LL_I2C_HandleTransfer(
             handle->bus->i2c,
@@ -108,18 +130,16 @@ bool furi_hal_i2c_rx(
                 *data = LL_I2C_ReceiveData8(handle->bus->i2c);
                 data++;
                 size--;
-            time_left = timeout;
             }
 
-        if(LL_SYSTICK_IsActiveCounterFlag()) {
+            if(osKernelGetTickCount() >= timeout_tick) {
-            if(--time_left == 0) {
                 ret = false;
                 break;
             }
         }
-    }
 
         LL_I2C_ClearFlag_STOP(handle->bus->i2c);
+    } while(0);
 
     return ret;
 }
@@ -132,6 +152,7 @@ bool furi_hal_i2c_trx(
     uint8_t* rx_data,
     uint8_t rx_size,
     uint32_t timeout) {
+
     if(furi_hal_i2c_tx(handle, address, tx_data, tx_size, timeout) &&
        furi_hal_i2c_rx(handle, address, rx_data, rx_size, timeout)) {
         return true;

firmware/targets/f6/furi-hal/furi-hal.c

@@ -17,6 +17,9 @@ void furi_hal_init() {
     furi_hal_interrupt_init();
     furi_hal_delay_init();
 
+    // FreeRTOS glue
+    furi_hal_os_init();
+
     MX_GPIO_Init();
     FURI_LOG_I(TAG, "GPIO OK");
 
@@ -56,9 +59,6 @@ void furi_hal_init() {
     furi_hal_bt_init();
     furi_hal_compress_icon_init();
 
-    // FreeRTOS glue
-    furi_hal_os_init();
-
     // FatFS driver initialization
     MX_FATFS_Init();
     FURI_LOG_I(TAG, "FATFS OK");

firmware/targets/f7/fatfs/stm32_adafruit_sd.c

@@ -405,7 +405,7 @@ BSP_SD_ReadBlocks(uint32_t* pData, uint32_t ReadAddr, uint32_t NumOfBlocks, uint
         goto error;
     }
 
-    ptr = malloc(sizeof(uint8_t) * BlockSize);
+    ptr = furi_alloc(sizeof(uint8_t) * BlockSize);
     if(ptr == NULL) {
         goto error;
     }
@@ -483,7 +483,7 @@ BSP_SD_WriteBlocks(uint32_t* pData, uint32_t WriteAddr, uint32_t NumOfBlocks, ui
         goto error;
     }
 
-    ptr = malloc(sizeof(uint8_t) * BlockSize);
+    ptr = furi_alloc(sizeof(uint8_t) * BlockSize);
     if(ptr == NULL) {
         goto error;
     }

firmware/targets/f7/furi-hal/furi-hal-i2c.c

@@ -46,12 +46,24 @@ bool furi_hal_i2c_tx(
     const uint8_t* data,
     uint8_t size,
     uint32_t timeout) {
-    furi_check(handle->bus->current_handle == handle);
-    uint32_t time_left = timeout;
-    bool ret = true;
 
-    while(LL_I2C_IsActiveFlag_BUSY(handle->bus->i2c))
+    furi_check(handle->bus->current_handle == handle);
-        ;
+    furi_assert(timeout > 0);
+
+    bool ret = true;
+    uint32_t timeout_tick = osKernelGetTickCount() + timeout;
+
+    do {
+        while(LL_I2C_IsActiveFlag_BUSY(handle->bus->i2c)) {
+            if(osKernelGetTickCount() >= timeout_tick) {
+                ret = false;
+                break;
+            }
+        }
+
+        if(!ret) {
+            break;
+        }
 
         LL_I2C_HandleTransfer(
             handle->bus->i2c,
@@ -66,18 +78,16 @@ bool furi_hal_i2c_tx(
                 LL_I2C_TransmitData8(handle->bus->i2c, (*data));
                 data++;
                 size--;
-            time_left = timeout;
             }
 
-        if(LL_SYSTICK_IsActiveCounterFlag()) {
+            if(osKernelGetTickCount() >= timeout_tick) {
-            if(--time_left == 0) {
                 ret = false;
                 break;
             }
         }
-    }
 
         LL_I2C_ClearFlag_STOP(handle->bus->i2c);
+    } while(0);
 
     return ret;
 }
@@ -88,12 +98,24 @@ bool furi_hal_i2c_rx(
     uint8_t* data,
     uint8_t size,
     uint32_t timeout) {
-    furi_check(handle->bus->current_handle == handle);
-    uint32_t time_left = timeout;
-    bool ret = true;
 
-    while(LL_I2C_IsActiveFlag_BUSY(handle->bus->i2c))
+    furi_check(handle->bus->current_handle == handle);
-        ;
+    furi_assert(timeout > 0);
+
+    bool ret = true;
+    uint32_t timeout_tick = osKernelGetTickCount() + timeout;
+
+    do {
+        while(LL_I2C_IsActiveFlag_BUSY(handle->bus->i2c)) {
+            if(osKernelGetTickCount() >= timeout_tick) {
+                ret = false;
+                break;
+            }
+        }
+
+        if(!ret) {
+            break;
+        }
 
         LL_I2C_HandleTransfer(
             handle->bus->i2c,
@@ -108,18 +130,16 @@ bool furi_hal_i2c_rx(
                 *data = LL_I2C_ReceiveData8(handle->bus->i2c);
                 data++;
                 size--;
-            time_left = timeout;
             }
 
-        if(LL_SYSTICK_IsActiveCounterFlag()) {
+            if(osKernelGetTickCount() >= timeout_tick) {
-            if(--time_left == 0) {
                 ret = false;
                 break;
             }
         }
-    }
 
         LL_I2C_ClearFlag_STOP(handle->bus->i2c);
+    } while(0);
 
     return ret;
 }
@@ -132,6 +152,7 @@ bool furi_hal_i2c_trx(
     uint8_t* rx_data,
     uint8_t rx_size,
     uint32_t timeout) {
+
     if(furi_hal_i2c_tx(handle, address, tx_data, tx_size, timeout) &&
        furi_hal_i2c_rx(handle, address, rx_data, rx_size, timeout)) {
         return true;

firmware/targets/f7/furi-hal/furi-hal.c

@@ -17,6 +17,9 @@ void furi_hal_init() {
     furi_hal_interrupt_init();
     furi_hal_delay_init();
 
+    // FreeRTOS glue
+    furi_hal_os_init();
+
     MX_GPIO_Init();
     FURI_LOG_I(TAG, "GPIO OK");
 
@@ -56,9 +59,6 @@ void furi_hal_init() {
     furi_hal_bt_init();
     furi_hal_compress_icon_init();
 
-    // FreeRTOS glue
-    furi_hal_os_init();
-
     // FatFS driver initialization
     MX_FATFS_Init();
     FURI_LOG_I(TAG, "FATFS OK");