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

Allow attribute get on root #7873

Merged
merged 4 commits into from
Mar 3, 2021

Conversation

qistoph
Copy link
Contributor

@qistoph qistoph commented Feb 12, 2021

This PR enables attributes on the root, such as creation/modification time of volume.

  fs::Dir fs = LittleFS.openDir("/");
  Serial.print("FS creation timestamp: ");
  Serial.println(fs.fileCreationTime());

After quite some testing I believe the attached changed is the only one required. It didn't seem to break anything in my tests, though I'm not 100% sure, because the use and value of _valid in undocumented and unclear to me.

Test project available: LittleFSTest

Effects of checking 'c' and 't' attributes on a root node, if:

  1. attribute present: return its value (mklittlefs since PR15)
  2. attribute unavailable at root and sub nodes: return 0 (older version of mklittlefs)
  3. attribute unavailable at root, but available at sub node: return (last?) sub node's value

I'm haven't been able to figure out why situation 3. doesn't respond like 2. If this is an undesirable side effect, I'm hoping someone has a suggestion on fixing it or where to look.

Copy link
Collaborator

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm worried that when _valid is false the filename is not guaranteed to be proper. Has this been verified?

Also, the ::format method should be setting these attributes when on-device formatting occurs.

@qistoph
Copy link
Contributor Author

qistoph commented Feb 13, 2021

I'll extend my tests and verify the workings of all available methods.

Where would you say is the right place to set the attributes on ::format?

  1. LittleFSImpl::format (LittleFS.h)
  2. lfs_format (lfs.c)

The first would be most in line with the change in mklittlefs, I guess.

@earlephilhower
Copy link
Collaborator

earlephilhower commented Feb 13, 2021

The LittleFSImpl::format() would be the spot to add the attributes. We don't touch the upstream FS lib itself.

The _valid thing still sets off alarms. You're assuming that dirent is all 0'd (which it is, luckily, in the constructor...but only until something writes to it which can happen on rewind() or next() or other calls). Also, this would return the FS date no matter what the subdir it's in which is not what you're going for. Currently, also, if I read the sprintf logic you're doing a lfs_get_attr("") and not lfs_get_attr("/")

@qistoph
Copy link
Contributor Author

qistoph commented Feb 17, 2021

Thanks for the feedback and insights. I have updated the code to a more proper way to get the attributes. Also included is the proposed change to set the attributes on format.

LittleFS.creationTime()

Copy link
Collaborator

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good way to add in the call! Just some minor things and we should be good to go!

libraries/LittleFS/src/LittleFS.h Outdated Show resolved Hide resolved
cores/esp8266/FSImpl.h Outdated Show resolved Hide resolved
libraries/LittleFS/src/LittleFS.h Outdated Show resolved Hide resolved
cores/esp8266/FS.cpp Outdated Show resolved Hide resolved
When `FS::getCreationTime` isn't implemented in the FS, return 0 not -1, same as other getXXXtime() calls in the File object.
@earlephilhower earlephilhower merged commit c90c329 into esp8266:master Mar 3, 2021
earlephilhower added a commit to earlephilhower/Arduino that referenced this pull request Mar 4, 2021
Rebuild entire toolchain instead of manually hacking the tools JSON to
ensure repeatability.

New mklittlefs sets the new FS timestamp added in esp8266#7873
d-a-v pushed a commit that referenced this pull request Mar 4, 2021
Rebuild entire toolchain instead of manually hacking the tools JSON to
ensure repeatability.

New mklittlefs sets the new FS timestamp added in #7873
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 this pull request may close these issues.

None yet

2 participants