FDS Write: Wrong Written Data

[jloliveira]

Good Afternoon,

I'm writing some config data on an empty record the first time I turn on my board and updating that same record every next time I turn on the board.

While the writing of the first data is correctly written with fds_record_write() - when I check the data with fds_record_open() -, the data is totally wrongly written when I update the record with new config data, with fds_record_update().

When the board initialises I initialise FDS and call flash_write_record_setup(&DEVICE_struct_w_config2write) after initialising the softdevice.

By looking into the code below, do you have any cue why the data is not correctly written when I update the record?

Thank you,

João Oliveira

/* Declarations in flash.h */

/* File ID and Key used for the configuration record. */

#define CONFIG_FILE     (0xF010)
#define CONFIG_REC_KEY  (0x7010)

typedef struct
{
	uint8_t enable;
	uint8_t time_adv;
}ADVERTISE;

typedef struct
{
	uint8_t state;
	ADVERTISE adv2;
}DEVICE;

/* flash_write_record_setup function in flash.c */

void flash_write_record_setup(DEVICE * config2write)
{
    ret_code_t rc;

    fds_record_desc_t desc = {0};
    fds_find_token_t  tok  = {0};

    //CONFIG_FILE and CONFIG_REC_KEY defined in flash.h
    rc = fds_record_find(CONFIG_FILE, CONFIG_REC_KEY, &desc, &tok);

    if (rc == FDS_SUCCESS) //record exists => update it
    {
        /* A config file is in flash. Let's update it. */
        fds_flash_record_t config = {0};

        /* Open the record and read its contents. */
        rc = fds_record_open(&desc, &config);
        APP_ERROR_CHECK(rc);

        /* Close the record when done reading. */
        rc = fds_record_close(&desc);
        APP_ERROR_CHECK(rc);
 
        //compare existing config in flash with new one
        uint8_t ret = memcmp(config2write, config.p_data, sizeof(DEVICE));

        if(ret != 0) //if different update config in flash
        {
            /* Write the updated record to flash. */
			fds_record_t const rec =
			{
				.file_id           = CONFIG_FILE,
				.key               = CONFIG_REC_KEY,
				.data.p_data       = config2write,
				.data.length_words = (sizeof(DEVICE) + 3) / sizeof(uint32_t)
			};

            rc = fds_record_update(&desc, &rec);
            APP_ERROR_CHECK(rc);

            m_write_finished = false;
        }
        else
        {
            /* Same config data as previous, nothing to be done */
        }
    }
    else  /* System config not found; write a new one. */
    {
        fds_record_t const rec =
        {
            .file_id           = CONFIG_FILE,
            .key               = CONFIG_REC_KEY,
            .data.p_data       = config2write,
            .data.length_words = (sizeof(DEVICE) + 3) / sizeof(uint32_t)
        };

        rc = fds_record_write(&desc, &rec);
        APP_ERROR_CHECK(rc);

        m_write_finished = false;
    }
}

[jloliveira]

Dear Rune,

 

Thank you for your reply, I also used a static variable to make sure it is word-aligned but still required to declare __ALIGN(4) in the DEVICE struct declaration itself in order to make sure it never returns FDS_ERR_UNALIGNED_ADDR when writing to flash, as flash_write_config_record() is called from different scopes (from different .c files).

In the meantime I also created read/write fds functions for a userid in char* format. As these are also called from different scopes, in order to guarantee no word-alignment errors, the flash_write_userid_record() function copies the userid2write data into an __ALIGN(4) static char* variable to be written into flash.

As per flash_read_userid(), I had to use a pointer to char* pointing to the flash data char* to return it.

 

Although every read/write call seem to work fine, for some reason I can't identify some rare times the app crashes when calling flash_read_userid() just after setting up the BLE connection without any assert, and the flash record seem to get corrupted as I can't write to it anymore unless I delete it before.

Is there anything I'm doing wrong with the read/write user id functions and calls, or could it be colliding with the BLE radio activity somehow?

 

Find the new code below:

 

/* Declarations in flash.h */

/* File ID and Key used for the configuration record. */

#define CONFIG_FILE     (0xF010)
#define CONFIG_REC_KEY  (0x7010)

/* File ID and Key used for the userid record. */

#define USERID_FILE     (0xF050)
#define USERID_REC_KEY  (0x7050)

typedef struct
{
	uint8_t enable;
	uint8_t time_adv;
}ADVERTISE;

typedef struct
{
	uint8_t state;
	ADVERTISE adv2;
}__ALIGN(4) DEVICE;


/* ========= flash.c ==========

ret_code_t flash_read_config(DEVICE * p_cfg)
{
    ret_code_t read_success = FDS_ERR_NOT_FOUND;

    fds_record_desc_t desc = {0};
    fds_find_token_t  tok  = {0};

    //Loop until all records with the given key and file ID have been found.
    while (fds_record_find(CONFIG_FILE, CONFIG_REC_KEY, &desc, &tok) == FDS_SUCCESS)
    {
        ret_code_t rc;
        fds_flash_record_t frec = {0};

        rc = fds_record_open(&desc, &frec);

        switch (rc)
        {
            case FDS_SUCCESS:
                break;

            case FDS_ERR_CRC_CHECK_FAILED:
                #ifdef _UART_FLASH_
                NRF_LOG_INFO("error: CRC check failed!");
                #endif
                continue;

            case FDS_ERR_NOT_FOUND:
                #ifdef _UART_FLASH_
                NRF_LOG_INFO("error: record not found!");
                #endif
                continue;

            default:
            {
                #ifdef _UART_FLASH_
                NRF_LOG_INFO("error: unexpected error %s.",
                                fds_err_str[rc]);
                #endif

                continue;
            }
        }

        //memcpy to pass p_cfg pointer with flash data from frec.p_data
        memcpy(p_cfg, frec.p_data, sizeof(DEVICE));

        NRF_LOG_INFO("FLASH READ CONFIG 00%x0%x0%x @ %x:",
        p_cfg->adv2.time_adv, p_cfg->adv2.enable, p_cfg->state, tok.p_addr);
        NRF_LOG_INFO("state: %d\r\n" 
                      "adv2.enable: %d\r\n"
                      "adv2.time_adv: %d\r\n",
                      p_cfg->state, p_cfg->adv2.enable, p_cfg->adv2.time_adv);

        rc = fds_record_close(&desc);
        APP_ERROR_CHECK(rc);

        read_success = FDS_SUCCESS; //if found and read record
    }

    return read_success;
}

void flash_write_config_record(DEVICE * config2write)
{
    ret_code_t rc;

    fds_record_desc_t desc = {0};
    fds_find_token_t  tok  = {0};

    // Copy write data to static word-aligned variable.
    uint32_t const len_cfg = sizeof(config2write);
    __ALIGN(4) static DEVICE flash_write_data;
    memcpy(&flash_write_data, config2write, len_cfg);

    //CONFIG_FILE and CONFIG_REC_KEY defined in flash.h
    rc = fds_record_find(CONFIG_FILE, CONFIG_REC_KEY, &desc, &tok);
    if (rc == FDS_SUCCESS) //record exists => update it
    {
        /* A config file is in flash. Let's update it. */
        fds_flash_record_t config = {0};

        /* Open the record and read its contents. */
        rc = fds_record_open(&desc, &config);
        APP_ERROR_CHECK(rc);

        //compare existing config in flash with new one      
        NRF_LOG_INFO("CONFIG IN FLASH:\r\n"
                  "state: %d\r\n" 
                  "adv2.enable: %d\r\n"
                  "adv2.time_adv: %d\r\n",
                  ((DEVICE*)config.p_data)->state, ((DEVICE*)config.p_data)->adv2.enable, ((DEVICE*)config.p_data)->adv2.time_adv);
        
        NRF_LOG_INFO("NEW CONFIG TO UPDATE:\r\n"
                      "state: %d\r\n" 
                      "adv2.enable: %d\r\n"
                      "adv2.time_adv: %d\r\n",
                      flash_write_data.state, flash_write_data.adv2.enable, flash_write_data.adv2.time_adv);

        uint8_t ret = memcmp(&flash_write_data, config.p_data, len_cfg);
        
        // Close the record when done reading. 
        rc = fds_record_close(&desc);
        APP_ERROR_CHECK(rc);
        
        if(ret != 0) //if different update config in flash
        {
            record_update(CONFIG_FILE, CONFIG_REC_KEY, desc, &flash_write_data, len_cfg);

            NRF_LOG_INFO("===== UPDATED FDS FLASH =====);

            m_write_finished = false;
        }
        else
        {
             #ifdef _UART_FLASH_
             NRF_LOG_INFO("FLASH: same config data as previous, nothing to be done:\r\n"
                      "state: %d\r\n" 
                      "adv2.enable: %d\r\n"
                      "adv2.time_adv: %d\r\n",
                      ((DEVICE*)config.p_data)->state, ((DEVICE*)config.p_data)->adv2.enable, ((DEVICE*)config.p_data)->adv2.time_adv);
             #endif
        }
    }
    else
    {
        /* System config not found; write a new one. */
        NRF_LOG_INFO("NEW CONFIG TO WRITE:\r\n"
                      "state: %d\r\n" 
                      "adv2.enable: %d\r\n"
                      "adv2.time_adv: %d\r\n",
                      flash_write_data.state, flash_write_data.adv2.enable, flash_write_data.adv2.time_adv);

        record_write(CONFIG_FILE, CONFIG_REC_KEY, &flash_write_data, len_cfg);

        NRF_LOG_INFO("===== WROTE FDS FLASH =====");

        m_write_finished = false;
    }
}

//NOTE: Need Pointer to Pointer for userid to return the char* in the address pointing to freq.p_data because it is an array of chars and therefore can't use memcpy to copy freq.p_data and return because it loses scope
static void record_write(uint32_t fid,
                         uint32_t key,
                         void const * p_data,
                         uint32_t len)
{
    fds_record_t const rec =
    {
        .file_id           = fid,
        .key               = key,
        .data.p_data       = p_data,
        .data.length_words = (len + 3) / sizeof(uint32_t)
    };

    #ifdef _UART_FLASH_
    NRF_LOG_INFO("--> writing record to flash...\r\n"
                    "file: 0x%x, key: 0x%x, \"%s\", len: %u bytes",
                    fid, key, p_data, len);
    #endif

    ret_code_t rc = fds_record_write(NULL, &rec);

    #ifdef _UART_FLASH_
    if (rc != FDS_SUCCESS)
    {
        NRF_LOG_INFO("--> error: fds_record_write() returned %s.",
                        fds_err_str[rc]);
    }
    #endif
}

static void record_update(uint32_t fid,
                         uint32_t key,
                         fds_record_desc_t desc, 
                         void const * p_data,
                         uint32_t len)
{
    fds_record_t const rec =
    {
        .file_id           = fid,
        .key               = key,
        .data.p_data       = p_data,
        .data.length_words = (len + 3) / sizeof(uint32_t)
    };

    #ifdef _UART_FLASH_
    NRF_LOG_INFO("--> updating record in flash...\r\n"
                    "file: 0x%x, key: 0x%x, \"%s\", len: %u bytes",
                    fid, key, p_data, len);
    #endif

    ret_code_t rc = fds_record_update(&desc, &rec);

    #ifdef _UART_FLASH_
    if (rc != FDS_SUCCESS)
    {
        NRF_LOG_INFO("--> error: fds_record_write() returned %s.",
                        fds_err_str[rc]);
    }
    #endif
}

ret_code_t flash_read_userid(char ** userid)
{
    ret_code_t read_success = FDS_ERR_NOT_FOUND;

    fds_record_desc_t desc = {0};
    fds_find_token_t  tok  = {0};

    //Loop until all records with the given key and file ID have been found.
    while (fds_record_find(USERID_FILE, USERID_REC_KEY, &desc, &tok) == FDS_SUCCESS)
    {
        ret_code_t rc;
        fds_flash_record_t frec = {0};

        rc = fds_record_open(&desc, &frec);
        switch (rc)
        {
            case FDS_SUCCESS:
                break;

            case FDS_ERR_CRC_CHECK_FAILED:
                #ifdef _UART_FLASH_
                NRF_LOG_INFO("error: CRC check failed!");
                #endif
                continue;

            case FDS_ERR_NOT_FOUND:
                #ifdef _UART_FLASH_
                NRF_LOG_INFO("error: record not found!");
                #endif
                continue;

            default:
            {
                #ifdef _UART_FLASH_
                NRF_LOG_INFO("error: unexpecte error %s.",
                                fds_err_str[rc]);
                #endif

                continue;
            }
        }

        //Memcpy frec.p_data address and point to copy's address
        uint32_t const len_str = strlen((char*)frec.p_data)+1; //NOTE: +1 to include NULL char in end of string
        __ALIGN(4) static uint8_t * flash_read_data;
        flash_read_data = malloc(sizeof(uint8_t) * len_str);
        memset(flash_read_data, 0x00, sizeof(flash_read_data));
        memcpy(flash_read_data, frec.p_data, len_str);
        
        *userid = (char*)flash_read_data;

        rc = fds_record_close(&desc);
        APP_ERROR_CHECK(rc);

        read_success = FDS_SUCCESS; //if found and read record
    }

    return read_success;
}

void flash_write_userid_record(char * userid2write)
{
    ret_code_t rc;

    fds_record_desc_t desc = {0};
    fds_find_token_t  tok  = {0};

    // Copy write data to static word-aligned variable.
    uint32_t const len_str = strlen(userid2write)+1; //NOTE: +1 to include NULL char in end of string
    __ALIGN(4) static char * flash_write_data;
    flash_write_data = malloc(sizeof(char) * len_str);
    memset(flash_write_data, 0x00, sizeof(flash_write_data));
    memcpy(flash_write_data, userid2write, len_str);

    //CONFIG_FILE and CONFIG_REC_KEY defined in flash.h
    rc = fds_record_find(USERID_FILE, USERID_REC_KEY, &desc, &tok);
    if (rc == FDS_SUCCESS) //record exists => update it
    {
        /// A userid file is in flash. Let's update it.
        fds_flash_record_t userid = {0};

        // Open the record and read its contents. 
        rc = fds_record_open(&desc, &userid);
        APP_ERROR_CHECK(rc);
 
        //compare existing config in flash with new one      
        //NOTE: need to copy flash data to static var to hold the data correctly, and __ALIGNED(4) to guarantee word-alignment
        uint32_t const len_str = strlen((char*)userid.p_data)+1; //NOTE: +1 to include NULL char in end of string
        __ALIGN(4) static char * flash_read_data;
        flash_read_data = malloc(sizeof(char) * len_str);
        memset(flash_read_data, 0x00, sizeof(flash_read_data));
        memcpy(flash_read_data, userid.p_data, len_str);
        

        //__ALIGN(4) static char * flash_read_data;
        //flash_read_data = (char*)userid.p_data;

        #ifdef _UART_FLASH_
        NRF_LOG_INFO("NEW USERID TO UPDATE: %s", userid2write);
        NRF_LOG_INFO("USERID IN FLASH     : %s", flash_read_data);
        #endif

        //NOTE: Use strlen to compare strings until end of first string
        uint8_t ret = memcmp(userid2write, flash_read_data, len_str);

        // Close the record when done reading. 
        rc = fds_record_close(&desc);
        APP_ERROR_CHECK(rc);

        if(ret != 0) //if different update config in flash
        {
            // Write the updated record to flash.
            #ifdef _UART_FLASH_            
            NRF_LOG_INFO("FLASH TO UPDATE W/ NEW USERID: %s", flash_write_data);       
            #endif

            record_update(USERID_FILE, USERID_REC_KEY, desc, flash_write_data, len_str);

            #ifdef _UART_FLASH_
            NRF_LOG_INFO("===== UPDATED FDS FLASH W/ USERID =====");
            #endif

            m_write_finished = false;
        }
        else
        {
             #ifdef _UART_FLASH_
             NRF_LOG_INFO("FLASH: same userid data as previous, nothing to be done");
             #endif
        }
    }
    else
    {
        // System config not found; write a new one.
        #ifdef _UART_FLASH_
        NRF_LOG_INFO("FLASH TO WRITE NEW USERID: %s", flash_write_data);
        #endif

        record_write(USERID_FILE, USERID_REC_KEY, flash_write_data, len_str);

        #ifdef _UART_FLASH_
        NRF_LOG_INFO("===== WROTE FDS FLASH =====");
        #endif

        m_write_finished = false;
    }
}

 

Thank you,

João