search

ruuvi / ruuvi.firmware.c

Ruuvi Firmware version 3. Built on top of Nordic SDK 15, uses both Ruuvi and external repositories as submodules. In Beta, no breaking changes are intended but may be done if absolutely necessary
BSD 3-Clause "New" or "Revised" License
52 stars 39 forks source link

app_log anomalies: first OR last time stamp is one or two !? less than it should be #308

Closed DG12 closed 2 years ago

DG12 commented 2 years ago

As time_stamp of last element at 72FBC is 1B7 should end_time be 1B7 rather than 1B6 ??

Beginning of first record:

J-Link>mem32 00072000 2C
00072000 = DEADC0DE F11E01FE 03EDF000 000000F0 //  FDS_PAGE_TAG(0..1) FDS_HEADER(0..1)
00072010 = 000007B0 00000000 000001B6 000000FA  // FDS_HEADER(2) ,   start_time, end_time, num_samples, 
00072020 = 00000000 00000000 00000000 41CFE979  // block_configuration;
00072030 = 41D976C9 47C41A8C 00000001 41CF8106 
00072040 = 41D953F8 47C41A8B 00000003 41CF147B 
00072050 = 41D8F5C3 47C41A68 00000004 41CF872B 
00072060 = 41D8ED91 47C41A5D 00000006 41CF2B02

end of first record:

J-Link>mem32 72F90 20
00072F90 = 41D58F5C 47C41054 000001B4 41D5353F 
00072FA0 = 41D6C6A8 47C4106C 000001B6 41D524DD 
00072FB0 = 41D7F9DB 47C4107A 000001B7 41D5C49C  << Last element 
00072FC0 = 41D8D0E5 47C41092 FFFFFFFF FFFFFFFF 
00072FD0 = FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF 
00072FE0 = FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF 
00072FF0 = FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF 
00073000 = DEADC0DE F11E01FE 03EDF001 000000F0

See also issue #309

ojousima commented 2 years ago

As time_stamp of last element at 72FBC is 1B7 should end_time be 1B7 rather than 1B6 ??

Yes it should, thanks.

[edit] This comes from https://github.com/ruuvi/ruuvi.firmware.c/blob/7ed3bc94cf92f8996a119be73f98c5191b2ec5e4/src/app_log.c#L237, sample is processed into block and flash before end timestamp is updated.

niksiboxi commented 2 years ago

@DG12 Hi! Could you please provide additional information on how to reproduce this issue?

DG12 commented 2 years ago

I was doing a in depth study of the history log C code and flash device when I found this. It's unlikely that can be observed since it may result in 1 data point missing out of 250. The nature of bluetooth broadcast advertisement is that it is expected to be missing packets. I will be correcting this soon just for the sake of correctness and I have several other changes in progress.

niksiboxi commented 2 years ago

@DG12 Thanks for the update. I noticed you already made some changes in this library, however you are planning to do some other changes too? Could you please share what sort of fixes you'll introduce into the code?

niksiboxi commented 2 years ago

@ojousima The Flash tests seems to be working just fine.

After status |= ri_flash_store_test (printfp); :

J-Link>mem32 65000 14
00065000 = DEADC0DE F11E01FE 00050000 00000002 
00065010 = 00000001 73616C46 65742068 64207473 
00065020 = 20617461 00000031 00050001 00000002 
00065030 = 00000002 73616C46 65742068 64207473 
00065040 = 20617461 70220032 FFFFFFFF FFFFFFFF

After status |= ri_flash_delete_test (printfp);

J-Link>mem32 65000 14
00065000 = DEADC0DE F11E01FE FFFFFFFF FFFFFFFF 
00065010 = FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF 
00065020 = FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF 
00065030 = FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF 
00065040 = FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF
DG12 commented 2 years ago

I don't know where this display of ri_flash_store_test 00065000 = DEADC0DE F11E01FE 00050000 00000002 and status |= ri_flash_delete_test (printfp); 00065000 = DEADC0DE F11E01FE FFFFFFFF FFFFFFFF

is from.

3.31.1 release? with my changes? a test session? CEEDLING?

OR how it relates the this particular anomaly, i.e. n app_log anomalies: last time stamp is one less than it should be

308

ojousima commented 2 years ago

how it relates the this particular anomaly, i.e. n app_log anomalies: last time stamp is one less than it should be

308

Unrelated, that one is caused by off-by-one error in logging data blocks to RAM before flash operation

DG12 commented 2 years ago

Looks like start time stamp is now one OR two ! less than it should be.

app_log_bad-first-timestamp

0006A000 = DEADC0DE F11E01FE 03EDF008 000000F0 0006A010 = 00000F8B 00000DCA 00000F83 000000FA 0006A020 = 00000000 00000000 00000DCB 41B8DFD3

00072000 = DEADC0DE F11E01FE 03EDF010 000000F0 00072010 = 00000F93 00001B97 00001D51 000000FA 00072020 = 00000000 00000000 00001B99 41C064C4

0006F000 = DEADC0DE F11E01FE 03EDF00D 000000F0 0006F010 = 00000F90 00001669 00001823 000000FA 0006F020 = 00000000 00000000 0000166B 41BB07B1

00070000 = DEADC0DE F11E01FE 03EDF00E 000000F0 00070010 = 00000F91 00001823 000019DD 000000FA 00070020 = 00000000 00000000 00001825 41BD4B4A

ojousima commented 2 years ago

This is to keep start and end timestamps of blocks consistent without gaps, new block starts where old ends