-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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 IRremote.cpp to improve debugging #258
Update IRremote.cpp to improve debugging #258
Conversation
This is a small change, and definitely an improvement. I simply improved the debugging by stating whether a check passed or failed, for easier identification in debug mode.
Further improved debug formatting, & added F macro to reduce RAM usage during prints.
very minor changes
@@ -41,14 +41,19 @@ | |||
// | |||
int MATCH (int measured, int desired) | |||
{ | |||
DBG_PRINT("Testing: "); | |||
DBG_PRINT(F("Testing: ")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does F("") do? I'm not a very advanced C programmer so can you please explain it to me? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's an Arduino PROGMEM macro, to cause the string constant to take up Flash memory only but not SRAM. See here at the bottom: http://playground.arduino.cc/Learning/Memory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without it, all printed string constants take both flash and RAM memory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For anything <2 chars it's a wash. Just using it takes a couple bytes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the detailed explanation. I will look into merging this soon. Once again thank you for the contribution
DBG_PRINT(measured, DEC); | ||
DBG_PRINT(" <= "); | ||
DBG_PRINTLN(TICKS_HIGH(desired), DEC); | ||
DBG_PRINT(F(" <= ")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When using F()
does the compiler optimize the exact same strings and only put them into the Flash once?
IF not, then we should look at a way to make that happen (not just here, but review throughout).
Also, using F()
For all the text is a great idea. The whole Library should probably have that change applied. Kudos to @ElectricRCAircraftGuy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: "When using F() does the compiler optimize the exact same strings and only put them into the Flash once?"
A: No, as far as I can tell, even if the strings are identical, they are put in the flash again each time you use the F() macro. It's not an ideal solution, just a better solution than not using F(). To put it in only once you'd have to manually do that, then manually call them out to print them using AVRLibc directly http://www.nongnu.org/avr-libc/user-manual/group__avr__pgmspace.html.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the strings are used several times each, why not put them in like this:
const char DBGtext_lteq[] PROGMEM = {" <= "};
const char DBGtext_pass[] PROGMEM = {"?; passed"};
const char DBGtext_fail[] PROGMEM = {"?; FAILED"};
Then output them like this:
DBG_PRINTLN(F(DBGtext_pass));
I am not sure if the F() should be used in the above or not.
Would this solve the duplicate string issue and gain that part of the PROGMEM back as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#261
…ino-IRremote into ElectricRCAircraftGuy-patch-1 merging #258
Some body help me add the library, I follow the instruction but can't found the folder with name IRremote, only two file IRremote.h and IRremote.cpp |
@reednoel4u If you need help please create a new issue. Thanks! |
…ino-IRremote into ElectricRCAircraftGuy-patch-1 merging Arduino-IRremote#258
This is a small change, and definitely an improvement. I simply improved the debugging by stating whether a check passed or failed, for easier identification in debug mode.