search

timmbogner / Farm-Data-Relay-System

A system that uses ESP-NOW, LoRa, and other protocols to transport sensor data in remote areas without relying on WiFi.
MIT License
507 stars 114 forks source link

Buffer log writes before saving them to the file. #56

Closed thefeiter closed 2 years ago

thefeiter commented 2 years ago

As discussed in #45 I messed around with buffering the log writes to SD and Flash.

This is to minimize package loss on file writes. In "normal" use it should workout fine, but when running the stresstest one can find some bottlenecks.

thefeiter commented 2 years ago

Atm I use the same buffer for SD and flash.

Stress test is set to delay 30 ms btw.

For USE_SD_LOG only package loss is at 5% (worse than without buffering, was 3%) For USE_FS_LOG only package loss is down to 17% (w/o buffer it was 80%)

Now I am thinking of using the buffer only for the LittleFS logging, since the SD logging package loss got worse. But this would add more complexity and redundancy to the code. What do you think @timmbogner ?

timmbogner commented 2 years ago

Sorry, I think I over-emphasized the importance of the stress test for your functionality. I have to look a little more when I'm at the computer, but it looks great! My biggest concern is that rapidly opening/closing a file on an SD could corrupt the filesystem of the card. That was my main thought behind buffering writes.

thefeiter commented 2 years ago

Awesome, then the buffering would be perfect to reduce rapid writes!

10.07.2022 20:18:32 Timm Bogner @.***>:

Sorry, I think I over-emphasized the importance of the stress test for your functionality. I have to look a little more when I'm at the computer, but it looks great! My biggest concern is that rapidly opening/closing a file on an SD could corrupt the filesystem of the card. That was my main thought behind buffering writes.

— Reply to this email directly, view it on GitHub[https://github.com/timmbogner/Farm-Data-Relay-System/pull/56#issuecomment-1179775216], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AK7SSGXMZR2G23KXXYR3GETVTMHXJANCNFSM53FKBODA]. You are receiving this because you authored the thread.[Verfolgungsbild][https://github.com/notifications/beacon/AK7SSGTOWTHB6ZQM6DBU2X3VTMHXJA5CNFSM53FKBODKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOIZI7B4A.gif]

timmbogner commented 2 years ago

This all looks really good, and I think it may work a general-use function in the future. One thing that I realized: since unix timestamps are just the number seconds since an arbitrary time/date, 'seconds_since_reset' is actually in the same format as the unix timestamp, yes? 🤯 Nice.

thefeiter commented 2 years ago

To prevent issues with the millis() rollover I suggest changing the buffer delay functionality to compare durations instead of timestamps (https://arduino.stackexchange.com/a/12588)

thefeiter commented 2 years ago

I set the buffer release timer to 10 seconds. I think it could be longer.

timmbogner commented 2 years ago

I'm a dum-dum and pulled the requests out of order again, but I think I fixed it!