Skip to content

Add File::availableForWrite API #68

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

Merged

Conversation

sandeepmistry
Copy link
Contributor

This can allow sketches to see if the write will be non-blocking.

@TravisBuddy
Copy link

Hey @sandeepmistry,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: 67138c70-c50a-11e9-8712-75d78f9b457f

if (cardCommand(CMD13, 0) || spiRec()) {
error(SD_CARD_ERROR_WRITE_PROGRAMMING);
goto fail;
if (sync) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cmaglie @facchinm this part is a bit dangerous, since we no longer wait for the result of the write command.

In the SD card I used for testing, the waitNotBusy would spin for 10 - 20ms.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should take extra care and understand how people really use this library (like continuous blocking writes or other patterns).
Said that, I love this patch!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we can only activate this non-blocking behaviour if someone calls availableForWrite(). This will allow older sketches to continue with the existing code path.

Stay tuned, I'll push some more changes later today.

@per1234
Copy link
Contributor

per1234 commented Aug 22, 2019

Regarding the Travis CI issue, I have verified that it is fixed by #67. That could be merged now if you want to get the CI tests working again.

@TravisBuddy
Copy link

Hey @sandeepmistry,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: 300f1130-c5bf-11e9-8aae-f1e5ed08699b

@TravisBuddy
Copy link

Hey @sandeepmistry,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: cd2a1270-c5c0-11e9-8aae-f1e5ed08699b

@TravisBuddy
Copy link

Hey @sandeepmistry,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: 0fb146e0-c5cb-11e9-8aae-f1e5ed08699b

@TravisBuddy
Copy link

Hey @sandeepmistry,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: 43272570-c5cc-11e9-8aae-f1e5ed08699b

@TravisBuddy
Copy link

Hey @sandeepmistry,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: e86a84d0-c5dd-11e9-8aae-f1e5ed08699b

@sandeepmistry sandeepmistry changed the title [DO NOT MERGE] Add File::availableForWrite API Add File::availableForWrite API Aug 23, 2019
@sandeepmistry
Copy link
Contributor Author

This is ready for review now :)

@sandeepmistry sandeepmistry merged commit 659ee9e into arduino-libraries:master Sep 9, 2019
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.

4 participants