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

Update ReadWrite.ino #73

Merged
merged 3 commits into from
Feb 24, 2020
Merged

Conversation

lorforlinux
Copy link
Contributor

There is an incompatibility issue for showing the initialization error message for the examples.

The example CardInfo.ino has this code (which i personally like)

if (!card.init(SPI_HALF_SPEED, chipSelect)) {
    Serial.println("initialization failed. Things to check:");
    Serial.println("* is a card inserted?");
    Serial.println("* is your wiring correct?");
    Serial.println("* did you change the chipSelect pin to match your shield or module?");
    while (1);
  } else {
    Serial.println("Wiring is correct and a card is present.");
  }

While the other example, like the ReadWrite.ino uses this code

if (!SD.begin(4)) {
    Serial.println("initialization failed!");
    while (1);
  }
  Serial.println("initialization done.");

It's a better idea to make one function in the library for doing this instead of writing similar code in all the examples. I will work on updating the library code if others agree with me on this :)

@TravisBuddy
Copy link

Travis tests have failed

Hey @deepaklorkhatri007,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

TravisBuddy Request Identifier: e9d27ce0-5572-11ea-850c-e5458713aaa0

@lorforlinux
Copy link
Contributor Author

Hi, how can I create a local setup for fixing the issue of formatting in examples?

@per1234
Copy link
Contributor

per1234 commented Feb 23, 2020

@deepaklorkhatri007 in most cases, including this one (trailing space on line 44 of examples/ReadWrite/ReadWrite.ino), just doing a Tools > Auto Format in the Arduino IDE will provide compliant formatting.

I think it would be nice to fix the trailing space in the example, but you don't need to worry about the non-compliant formatting in src/utility/SdFat.h, as that was not caused by your PR.


Just in case you're interested in the details, or don't want to use the Arduino IDE:

The same formatter tool is used for this CI build as the Arduino IDE: Artistic Style. You can download it here:
https://sourceforge.net/projects/astyle/files
installation instructions here:
http://astyle.sourceforge.net/install.html

The configuration file we use for the CI builds is available here:
https://raw.githubusercontent.com/arduino/Arduino/master/build/shared/examples_formatter.conf

The documentation for Artistic Style is here:
http://astyle.sourceforge.net/astyle.html

If you wanted to format all .ino files recursively from the current directory, using the examples_formatter.conf configuration file, without making backups, the command would look something like this:

astyle --suffix=none --options="examples_formatter.conf" --recursive  ./*.pde,*.ino

@lorforlinux
Copy link
Contributor Author

Hi @per1234, thank you so much for the details.

I think it would be nice to fix the trailing space in the example, but you don't need to worry about the non-compliant formatting in src/utility/SdFat.h, as that was not caused by your PR.

yes, the build was failing before my pull request.

just doing a Tools > Auto Format in the Arduino IDE will provide compliant formatting.

I will try to fix the issue :)

@lorforlinux
Copy link
Contributor Author

All checks are passing now, I am using Fedora Linux and astyle is already installed on my machine. If it was not installed i may have used

sudo dnf install astyle

For fixing the formatting we can directly use the command suggested by @per1234 (I did it one by one to get to know the tool better)

For formatting all the files from in SD/ directory run

$ cd SD/
$ astyle --suffix=none --options="examples_formatter.conf" --recursive  ./*.pde,*.ino -v

For doing it one by one run

$ cd {folder}
$ astyle --suffix=none --options="examples_formatter.conf" --recursive  {file}.ino -v

Now I want to fix the code redundancy of the error message if everybody agrees on it. Basically I am going to create one function which we can use to see if the initialization is successful or not.

Copy link
Contributor

@aentinger aentinger left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Thank you @deepaklorkhatri007 for your contribution 😉

@aentinger aentinger merged commit 4d02d9b into arduino-libraries:master Feb 24, 2020
@per1234 per1234 added type: enhancement Proposed improvement topic: documentation Related to documentation for the project labels Apr 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: documentation Related to documentation for the project type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants