BLE+FDS Problem to read/write flash data err

[kenyon]

sdk：17.1
example: ble_app_uart+ secure_bootloader.
Our device is charged with power-off reset function.

When I call fds_record_update(&desc, &gtPara_record),if returns FDS_ERR_NO_SPACE_IN_FLASH, which means I need to call to clean up the garbage.So i need to call err_code = fds_gc();Right？

// <e> FDS_ENABLED - fds - Flash data storage module
//==========================================================
#ifndef FDS_ENABLED
#define FDS_ENABLED 1
#endif
// <h> Pages - Virtual page settings

// <i> Configure the number of virtual pages to use and their size.
//==========================================================
// <o> FDS_VIRTUAL_PAGES - Number of virtual flash pages to use. 
// <i> One of the virtual pages is reserved by the system for garbage collection.
// <i> Therefore, the minimum is two virtual pages: one page to store data and one page to be used by the system for garbage collection.
// <i> The total amount of flash memory that is used by FDS amounts to @ref FDS_VIRTUAL_PAGES * @ref FDS_VIRTUAL_PAGE_SIZE * 4 bytes.

#ifndef FDS_VIRTUAL_PAGES
#define FDS_VIRTUAL_PAGES 2
#endif

// <o> FDS_VIRTUAL_PAGE_SIZE  - The size of a virtual flash page.
 

// <i> Expressed in number of 4-byte words.
// <i> By default, a virtual page is the same size as a physical page.
// <i> The size of a virtual page must be a multiple of the size of a physical page.
// <1024=> 1024 
// <2048=> 2048 

#ifndef FDS_VIRTUAL_PAGE_SIZE
#define FDS_VIRTUAL_PAGE_SIZE 1024
#endif

// <o> FDS_VIRTUAL_PAGES_RESERVED - The number of virtual flash pages that are used by other modules. 
// <i> FDS module stores its data in the last pages of the flash memory.
// <i> By setting this value, you can move flash end address used by the FDS.
// <i> As a result the reserved space can be used by other modules.

#ifndef FDS_VIRTUAL_PAGES_RESERVED
#define FDS_VIRTUAL_PAGES_RESERVED 0
#endif

// </h> 
//==========================================================

// <h> Backend - Backend configuration

// <i> Configure which nrf_fstorage backend is used by FDS to write to flash.
//==========================================================
// <o> FDS_BACKEND  - FDS flash backend.
 

// <i> NRF_FSTORAGE_SD uses the nrf_fstorage_sd backend implementation using the SoftDevice API. Use this if you have a SoftDevice present.
// <i> NRF_FSTORAGE_NVMC uses the nrf_fstorage_nvmc implementation. Use this setting if you don't use the SoftDevice.
// <1=> NRF_FSTORAGE_NVMC 
// <2=> NRF_FSTORAGE_SD 

#ifndef FDS_BACKEND
#define FDS_BACKEND 2
#endif

// </h> 
//==========================================================

// <h> Queue - Queue settings

//==========================================================
// <o> FDS_OP_QUEUE_SIZE - Size of the internal queue. 
// <i> Increase this value if you frequently get synchronous FDS_ERR_NO_SPACE_IN_QUEUES errors.

#ifndef FDS_OP_QUEUE_SIZE
#define FDS_OP_QUEUE_SIZE 4
#endif

// </h> 
//==========================================================

// <h> CRC - CRC functionality

//==========================================================
// <e> FDS_CRC_CHECK_ON_READ - Enable CRC checks.

// <i> Save a record's CRC when it is written to flash and check it when the record is opened.
// <i> Records with an incorrect CRC can still be 'seen' by the user using FDS functions, but they cannot be opened.
// <i> Additionally, they will not be garbage collected until they are deleted.
//==========================================================
#ifndef FDS_CRC_CHECK_ON_READ
#define FDS_CRC_CHECK_ON_READ 1
#endif
// <o> FDS_CRC_CHECK_ON_WRITE  - Perform a CRC check on newly written records.
 

// <i> Perform a CRC check on newly written records.
// <i> This setting can be used to make sure that the record data was not altered while being written to flash.
// <1=> Enabled 
// <0=> Disabled 

#ifndef FDS_CRC_CHECK_ON_WRITE
#define FDS_CRC_CHECK_ON_WRITE 0
#endif

I have two questions about that.
1. How long does it take to clean fds gc.
2. What happens if the fds_gc is interrupted by a charging reset during cleanup?

Best regard
Kenyon

[Hung Bui]

Hi Zicongkuang, 

1. It depends. The gc process basically erase all the records that's marked as "dirty" . It does it by copy the "valid" records to the swap page, erase the page and copy the valid record to the current page. So the process may take longer time if you have a lot of pages to copy&erase. 
As mentioned in the documentation, you should wait for the FDS_EVT_GC event with NRF_SUCCESS status before continue. But I don't think it would take very long time (usually less than a second). You can find the timing specification here: https://docs.nordicsemi.com/bundle/ps_nrf52810/page/nvmc.html#unique_728973123

2. There is a checksum at each step in gc process. A page is backed up before it's erased. So there shouldn't be a situation when you lose your data when interrupted when doing gc. I don't think the GC process automatically resume after a reset, so you may need to call it again if you run out of flash again. 

It's a good practice to use  fds_stat to monitor the amount of flash available instead of waiting for FDS_ERR_NO_SPACE_IN_FLASH error. 

[kenyon]

Hi Hung Bui


After I used the FDS module, there was a problem found that there were two valid flash data in the flash area of the hex file, why is this, is there a solution? Or how can we avoid it？

Best Regard
Kenyon

[kenyon]

Hi Hung Bui

#define CONFIG_FILE (0x8010)
#define CONFIG_REC_KEY (0x7010)
static fds_record_t const gtPara_record =
{
.file_id = CONFIG_FILE,
.key = CONFIG_REC_KEY,
.data.p_data = &gtStorePara,
/* The length of a record is always expressed in 4-byte units (words). */
.data.length_words = (sizeof(gtStorePara) + sizeof(uint32_t) - 1) / sizeof(uint32_t),
};

We are constantly updating data under the same File ID and Record Key. So I think in flash we should only have one valid data. But strangely enough, we found two valid bits of data in flash using the hex file. Why is that?
Because of the existence of two valid data, we have to read the data two times each time to read out the latest update data.How can I avoid this?

Best regard
Kenyon

[Hung Bui]

Hi Kenyon, 

Have you managed to reproduce the issue ? If not please double check the code to see if there is any chance that you created two record with the same key ? 
How did you find out the problem ? Did it has any relation to a DFU update ? 

[kenyon]

Hi Hung Bui
I can reproduce the issue，maybe need a long time to reproduce.
Because the charging port of our device is equipped with hardware reset function，I reproduce this issue  by continuously charging reset(write flash) + key_longPress() (write flash).

#include <string.h>
#include "fds.h"

unsigned long gulFlashStoreNeeded = 0;////flash_store();                            
unsigned long gulFlashStoreCnt = 0x80000000;

#define CONFIG_FILE     (0x8010)
#define CONFIG_REC_KEY  (0x7010)
T_Para gtPara;

extern T_LL_HardwareID gtHwID;
///* A dummy structure to save in flash. */
typedef struct
{
    unsigned long p_v;
    T_DeviceLumen temote;
    signed char scBleAddr[6];
    E_Mode eMOWL; //
    unsigned long ulB;
    unsigned long eBmo;
    unsigned long ulNeedCharge;
    signed short sxyz[3];
    signed short ssThd;
    unsigned short usAdcValue;
    unsigned char aucSN[20];
    unsigned char ucBLight;
    E_B_M_O_P_O_O b_m_f;
    unsigned long flash_erase_flag;
    #define GET_BAR_STATE(states, bar_index)    ((states>>(bar_index<<1)) & 0x3) // 2 bit for each bar
    unsigned long c_f_t_s[6][7];   
    unsigned char brightness[6];    
    unsigned long ulChecksum;
    unsigned char publicNet;
    unsigned char netID[4];
    #define getCustomizedSetting(xxx)  gtPara.xxx
	unsigned char Profiles[4];
	unsigned char mode;
	unsigned char profile_nn;
    unsigned char netID_e;
    unsigned char PowerOff;
    unsigned char enabledHallSensor;    
}T_Para; //284 bytes

static T_Para gtStorePara = 
{
	.p_v = 0x01010102,
	.temote.ulRoundRobin = 0,
	.temote.ulIng        = 0,
	.temote.ID[0] = 0x00,
	.temote.ID[1] = 0x00,
	.temote.bitsc = 0x00,
	.eMOWL = 0,
	.ulB = 0,
	.eBmo = 0x02,
	.ulNeedCharge = 0,
	.sxyz[0] = 100,
	.sxyz[1] = 0,
	.sxyz[2] = 0,
	.ssThd = -3500,
	.ucBLight = 70,
	.publicNet = 0,
	.netID_e = 0,
	.PowerOff = 0,
	.b_m_f = 0x06,
};


/* A record containing dummy configuration data. */
static fds_record_t const gtPara_record =
{
    .file_id           = CONFIG_FILE,
    .key               = CONFIG_REC_KEY,
    .data.p_data       = &gtStorePara,
    /* The length of a record is always expressed in 4-byte units (words). */
    .data.length_words = (sizeof(gtStorePara) + sizeof(uint32_t) - 1) / sizeof(uint32_t),
};


static bool volatile m_fds_initialized;
static bool volatile m_fds_writeflag;
static bool volatile m_fds_gc_done = true;

static void fds_evt_handler(fds_evt_t const * p_evt)
{

    switch (p_evt->id)
    {
        case FDS_EVT_INIT:
            if (p_evt->result == NRF_SUCCESS)
            {
                m_fds_initialized = true;
            }
            break;

        case FDS_EVT_WRITE:
        case FDS_EVT_UPDATE:
        {
            if (p_evt->result == NRF_SUCCESS)
            {
				m_fds_writeflag = true;
            }                
        }break;
        case FDS_EVT_DEL_RECORD:
        {
            if (p_evt->result == NRF_SUCCESS)
            {

            }
            //m_delete_all.pending = false;
        } break;
        case FDS_EVT_GC:
            if (p_evt->result == NRF_SUCCESS)
            {
                LL_LED_OFF(E_LL_LED_PCB_G);
                m_fds_gc_done = true;
            }
        default:
            break;
    }
}


void flash_init(void)
{
   ret_code_t err_code;
    
    //write_flag init
    m_fds_writeflag = true;
    
   (void) fds_register(fds_evt_handler);//FDS register
    err_code = fds_init();//fds init
	
    APP_ERROR_CHECK(err_code);
	
	while (!m_fds_initialized)//wait init
    {
        sd_app_evt_wait();//standby
    }		
		
	fds_stat_t stat = {0};
    err_code = fds_stat(&stat);//Set statistics
    APP_ERROR_CHECK(err_code);
	
	//record_delete_next();//clear all records
			
	fds_record_desc_t desc = {0};//The descriptor structure used to manipulate the record is zeroed out
    fds_find_token_t  tok  = {0};//The token that saves the key is cleared
		
    err_code = fds_record_find(CONFIG_FILE, CONFIG_REC_KEY, &desc, &tok);//Find data, file ID, record KEY
		
		
 if (err_code == NRF_SUCCESS)//if find successful
    {
        /* A config file is in flash. Let's update it. */
        fds_flash_record_t config = {0};//res

        /* Open the record and read its contents. */
        err_code = fds_record_open(&desc, &config);//open record to read data

        APP_ERROR_CHECK(err_code); 
        /* Copy the configuration from flash into m_dummy_cfg*/
        memcpy(&gtPara, config.p_data, sizeof(gtPara));

        //data = (uint32_t *)config.p_data;
                
        /* Close the record when done reading. */
        err_code = fds_record_close(&desc);//close record
        APP_ERROR_CHECK(err_code);
    }else{
    	set_default; //set all data default
    	gulFlashStoreNeeded = 1; //store data
	}			
}

bool flash_gc_finish(void)
{
    return m_fds_gc_done;
}

bool flash_store_finish(void)
{
    return m_fds_writeflag;
}

void flash_store(void)
{
		ret_code_t err_code;
		fds_record_desc_t desc = {0};//The descriptor structure used to manipulate the record is zeroed out
        fds_find_token_t  tok  = {0};//The token that saves the key is cleared
   
        memcpy(&gtStorePara, &gtPara, sizeof(gtPara));
        
        //write_flag set
        m_fds_writeflag = false;
		// Check the record exists
		err_code = fds_record_find( CONFIG_FILE, CONFIG_REC_KEY, &desc, &tok );
		
		/* Write the updated record to flash.*/
		if( err_code == NRF_SUCCESS){
            err_code = fds_record_update(&desc, &gtPara_record);
            if(err_code == FDS_ERR_NO_SPACE_IN_FLASH){
                m_fds_gc_done = false;
                LL_LED_ON(E_LL_LED_PCB_G);
                err_code = fds_gc();
                APP_ERROR_CHECK(err_code);
                
                gulFlashStoreNeeded = 1; //store data
            }               
    
            fds_stat_t stat = {0};
            err_code = fds_stat(&stat);
            APP_ERROR_CHECK(err_code);				
		}else{
            err_code = fds_record_write(&desc, &gtPara_record);//Write records and data. Descriptor, used later to access the record ID
            APP_ERROR_CHECK(err_code);					
		}			
}

#define LL_FLASH_WAITING_TIME_BEFORE_SLEEP  (98 + 338*1024/1000 + 1280) + 1000    // 98ms page-erase, 338us 4-byte-write, 1024 4-byte per page, 1280ms top-prior-BLE-adv. (refer to "chapter 9 Flash memory API" of "S132_SDS_v2.0.pdf")
void LL_Power_BeforeSleep(void)
{
    //Store data and write crc checks again before sleep if charge or some data has not written
    if(gulFlashStoreNeeded){
        flash_store();
    }	
    
    //wait flash write success 
    if(!flash_store_finish()){ 
        for(uint16_t i=0; i<LL_FLASH_WAITING_TIME_BEFORE_SLEEP; i++){
            if(flash_store_finish()){break;}
            nrf_delay_ms(1);
        }
    }
}

int main(void)
{
    bool erase_bonds;
    ret_code_t err_code;
		
    // Initialize the async SVCI interface to bootloader before any interrupts are enabled.
    err_code = ble_dfu_buttonless_async_svci_init();
    APP_ERROR_CHECK(err_code);
   
    // Initialize.
    timers_init();

    ble_stack_init();
    gap_params_init();
    gatt_init();
    services_init();

    advertising_init();
    conn_params_init();
		//
#ifdef  _LL_TIMESLOT
    err_code = esb_timeslot_init();
    APP_ERROR_CHECK(err_code);
	
    err_code = esb_timeslot_sd_start();
    APP_ERROR_CHECK(err_code);
#endif

	LL_Key_Init();
		
    LL_Battery_Charging__Init();
        
	flash_init();

    // system off if not from key trigger or charging.
	if(LL_Battery_Charge__isCharging()) {  //if charging
        gtPara.ulNeedCharge = 0; gulFlashStoreNeeded = 1;  //store gtPara.ulNeedCharge
        LL_Power_BeforeSleep(); 
        LL_Power_Sleep(); 
    }else if( key_shortPress() ) {		// if key pressed
       	// continue			       
    }else{
        LL_Power_BeforeSleep(); 
        sd_power_system_off();            
    }
    
	// Start execution.
    advertising_start();//	
	
	while(1)
	{
		if(gulFlashStoreNeeded == 1){
			gulFlashStoreNeeded = 2;
			TimeStart(0);//Start the clock here 
		}else if(gulFlashStoreNeeded == 2){
			if(TimeStart(2000)) {//If you time it for 2000ms, start storing data
				flash_store();	
				gulFlashStoreNeeded = 0; //Wait for the next data store
			}
		}
		
		if( key_longPress()) { //if key longPress
			gtPara.ulNeedCharge = 1; gulFlashStoreNeeded = 1;
        	LL_Power_BeforeSleep(); 
        	sd_power_system_off();
		}
	}
}

When I checked thoroughly today, I found that it was because I did not know when I called err_code = fds_record_write(&desc, &gtPara_record) twice; The result is that two pieces of data are stored under the same KEY and File ID.
In my code, the condition for calling fds_record_write is err_code = fds_record_find(CONFIG_FILE, CONFIG_REC_KEY, &desc, &tok); fails to call fds_record_write again.

I wonder if the constant charging reset will cause this situation in flash？

[Hung Bui]

Hi Kenyon, 

To make sure you are not calling write twice, you should have a semaphore to wait for the event FDS_EVT_WRITE in fds_evt_handler() to be success before you call write again. 

I don't think doing reset multiple times when writing can cause the problem. If the record is written correctly before the reset, then the next fds_record_find() should be able to find it. If the writing is not finished, the CRC will not match.

[kenyon]

Hi Hung Bui

Hung Bui said:

To make sure you are not calling write twice, you should have a semaphore to wait for the event FDS_EVT_WRITE in fds_evt_handler() to be success before you call write again. 

This, in my opinion, only prevents two consecutive calls to fds_write caused by faulty software logic. Because the act of charging RESET is uncontrollable and random.Is it right？

#ifndef FDS_CRC_CHECK_ON_READ
#define FDS_CRC_CHECK_ON_READ 1
#endif

#ifndef FDS_CRC_CHECK_ON_WRITE
#define FDS_CRC_CHECK_ON_WRITE 0
#endif

I only turned on the read CRC，I may need to turn on write crc as well.
But I have a question: if there is no CRC write check, will fds_update also probably result in two sets of valid data in the same KEY &&FILE ID?

[Hung Bui]

Hi, 

I don't think turning that on would prevent the duplicate write issue. The setting is to check if the data writen has been modified when writing. If you have a reset in the middle of the writing process, in theory the data should just be marked as non-valid (not marked as valid). 
When you turn on FDS_CRC_CHECK_ON_READ  a corrupted record will still can be found but not open. Have you make sure you can open the duplicated records ? 
As I mentioned, please try to check and make sure you are not calling write fds_record_write() twice for the same record. 

[kenyon]

Hi
In the test I did just now, I didn't find that they called fds_write the second time, but I did find two more valid data in flash.

[Hung Bui]

Hi again, 
How long did it take to reproduce the issue ? 
Can you reproduce it on a DK ? If you can send a minimal sample that I can test here I can try to investigate. 

[kenyon]

Hi Hung Bui

I made a DK and was able to reproduce the problem on the development board (52832).
I found that the call collision of fds_update caused two valid data to appear instead of fds_write. If there are any errors in the code, please correct, thank you very much.

If fds_update does cause two valid pieces of data, how can we avoid it?


TestFlash_sdk17.7z

[Hung Bui]

Hi, 
Thanks for the code. I will take a look.
Could you give a brief info on how to test it with your code ? 
Could you elaborate more on " call collision of fds_update " ? 
The way it works is that fds_update will write the content of the new record to a new location in flash and mark the old one as invalid. There could be a chance that if the reset occurs right after the new data written in flash but the old record has not been marked as dirty then it will cause duplicated records. 
I will check internally to see if it's the case and if we have a solution.