Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Periodic persisting of log records fails #10

Closed
eupn opened this issue Jun 10, 2021 · 5 comments · Fixed by #11
Closed

Periodic persisting of log records fails #10

eupn opened this issue Jun 10, 2021 · 5 comments · Fixed by #11

Comments

@eupn
Copy link
Contributor

eupn commented Jun 10, 2021

Hello @MathiasKoch! Thanks for this crate, I find it useful and this is exactly what I had in mind for my project for quite some time!

I'm currently trying to plug this into my project that uses an STM32 MCU that has a typical FLASH controller that requires the page to be erased to be written to.

From my understanding, this requirement poses a challenge, that it isn't possible to periodically call drain_storage to flush records into the FLASH as it will fail on consequent calls due to the page with the header not being erased.

Currently, I can imagine some solutions to this:

  1. Keep the header in a separate FLASH page and erase it every time the header needs to be updated. This solution is pretty lame and wasteful of the FLASH resource but could work in the short term, or
  2. Do not use the header and rely on the fact that "empty" data in FLASH are always 0xFFs. Probably, the log records could have some magic number at the beginning (and/or the end) and during the write, we will skim through the log area to find an empty spot that is only 0xFFs and doesn't have that magic number at the beginning, or
  3. Use some more advanced data structure that is more suitable for a FLASH memory, probably similar to those used in filesystems

Hope it makes sense. What are your thoughts on this?

@MathiasKoch
Copy link
Member

Hi @eupn,
Thanks for reaching out!

It's been a while since i've had time to work on this, but i will soon have some time to give this crate the remaining love it requires. I think one of the main blockers for this crate to be really worth, is for defmt to implement framing into their wire stream. This seem to happening with the next breaking release: knurling-rs/defmt#492

Regarding your current issue, i did not think about using the internal flash as storage when i designed this, but i definetely think it should be a goal of this crate. I am not super fond of the header method myself, but couldn't quite think of any other way that would work as well. Your idea number 2 or 3 is worth looking into.

I think the first step would be a simple storage overhead calculation on idea number 2, with the STM32 ECC in mind, it migth still require a lot of read-modify-write operations, but that might be okay.

Regarding option 3, if you have knowledge of anything that would fit, i'd be happy to read up on it.

Also any contributions are welcomed! And i am also super open to maintainers, if you are going to use this broadly.

@eupn
Copy link
Contributor Author

eupn commented Jun 12, 2021

@MathiasKoch

Am I right to say that, until the defmt's framing implemented, we have to do it by ourselves by using, for example, COBS?

I think the first step would be a simple storage overhead calculation on idea number 2, with the STM32 ECC in mind, it migth still require a lot of read-modify-write operations, but that might be okay.

Yes, the STM32 FLASH's ECC is exactly the reason why it isn't possible to make writes of words less than 64 bit width. For example, if the frame is 5 bytes, then we have to skip 8 - 5 = 3 bytes before writing the next frame. And these 3 bytes would be wasted. As you may know, for some FLASH controllers it's possible to write zeros to bits that were erased as 1 and it might be of some use here, but when ECC is present, that's not the case.

I'm currently implementing a simple header-less approach that writes frames one after another while respecting the 8-byte word boundary and fills unused space with zeros so COBS decoder should just skip that.

Regarding the time overhead, we'll definitely have a O(n) time consumed during the init since we have to find a first vacant 64-bit word and set it as a head to write a next frame to. On a completely empty storage area i'll be negligible. After some logs are written, should restart happen and we have to re-initialize the logger, it'll start to consume O(n) time once for every restart. Space-wise, a single 64-bit word will be used as a magic number at the beginning of the storage area to signify that the area isn't empty. COBS aside with this approach, some bytes will be wasted for frames which aren't of length of a multiples of 8 (len % 8 != 0) due to ECC requirement described above.

Regarding option 3, if you have knowledge of anything that would fit, i'd be happy to read up on it.

I can only remember the SPIFFS file system what uses some wear-leveling as well as exploits the properties of a EEPROM/NOR-FLASH such as the ability to zero erased bits. But for logging use case this is definitely an overkill and also might not work for an internal FLASH due to ECC. I'm looking at the relevant literature (1, 2) for insights, but can't find a perfect match for our use case.

@MathiasKoch
Copy link
Member

Am I right to say that, until the defmt's framing implemented, we have to do it by ourselves by using, for example, COBS?

Problem is that this is not quite possible currently, due to the internals of how defmt works. Defmt is allowed to split log messages into multiple calls to write, which will break the way this crate currently works, as we have no way of implementing framing, other than "one call to write = one frame". In practise this probably wont happen too frequently, as i think it's mainly some advanced implementations of Format that uses this, but once it happens it will break the full log stream.

This multi-write pattern is also the reason they are implementing it as reverse-COBS, because they do not have the full length up front.

I think the overhead sounds reasonable to me.

I'm currently implementing a simple header-less approach that writes frames one after another while respecting the 8-byte word

That sounds great!

Ahh, yeah SPIFFS is probably overkill for this.

I have a PR that updates this to the latest defmt. I can try to put in half an hour first thing next week, to get that finished and merged.

@eupn eupn mentioned this issue Jun 13, 2021
@Dirbaio
Copy link

Dirbaio commented Jun 13, 2021

Problem is that this is not quite possible currently, due to the internals of how defmt works. Defmt is allowed to split log messages into multiple calls to write

You can: defmt does one acquire+release per frame, you can assume all the writes between an acquire and a release are one frame.

This multi-write pattern is also the reason they are implementing it as reverse-COBS, because they do not have the full length up front.

You can encode messages with unknown upfront length with both COBS and rCOBS. The advantage of rCOBS is that you don't need a 256-byte lookahead buffer.

@MathiasKoch
Copy link
Member

Aha. TIL on both of those!

@Dirbaio It's great to have you to enlighten me everywhere, thanks 👍 Keep it coming!

In that case, disregard my think about the new defmt being the current blocker.
@eupn Your PR looks great at first sight! I will try to review it and test it beginning of this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants