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

The example code in the Arduino UNO R4 WiFi Real-Time Clock document is incorrect and misleading #1351

Closed
eMUQI opened this issue Sep 19, 2023 · 2 comments

Comments

@eMUQI
Copy link

eMUQI commented Sep 19, 2023

Hello everyone, when I tried to run the example in https://docs.arduino.cc/tutorials/uno-r4-wifi/rtc#Periodic-Interrupt, I found some errors in it.

### Periodic Interrupt
A periodic interrupt allows you to set a recurring callback.
To use this, you will need to initialize the periodic callback, using the `setPeriodicCallback()` method:
- `RTC.setPeriodicCallback(periodic_cbk, Period::ONCE_EVERY_2_SEC)`
You will also need to create a function that will be called:
- `void periodic_cbk() { code to be executed }`
The example below blinks a light every 2 seconds:
```arduino
#include "RTC.h"
const int LED_ON_INTERRUPT = 22;
void setup(){
RTC.begin();
if (!RTC.setPeriodicCallback(periodic_cbk, Period::ONCE_EVERY_2_SEC)) {
Serial.println("ERROR: periodic callback not set");
}
}
void loop() {
}
void periodic_cbk() {
static bool clb_st = false;
if(clb_st) {
digitalWrite(LED_ON_INTERRUPT,HIGH);
}
else {
digitalWrite(LED_ON_INTERRUPT,LOW);
}
clb_st = !clb_st;
Serial.println("PERIODIC INTERRUPT");
}
```

Here are some issues I believe exist in the code and my suggestions:

  1. Pin error, Arduino R4 onboard LED is 13 or LED_BUILTIN, but code uses 22.
  2. Missing Serial.begin(9600);.
  3. Missing RTC.setTime(). According to RTC_PeriodicExample.ino RTC.setTime() must be called for RTC.setPeriodicCallback to work.
  4. Using camel case naming convention and more intuitive naming methods would be better. Refer to the Arduino Style Guide for Creating Libraries. The code mixes camel case naming convention and underscore naming convention. And names like periodic_cbk and clb_st confuse me, perhaps it would be better to not use abbreviations and instead use periodicCallback and callbackStatus?
    **Use full, everyday words.** Don’t be terse with your function names or variables. Use everyday terms instead of technical ones. Pick terms that correspond to popular perception of the concept at hand. Don’t assume specialized knowledge. For example, this is why we used `analogWrite()` rather than `pwm()`. Abbreviations are acceptable, though, if they’re in common use or are the primary name for something. For example, “HTML” is relatively common and “SPI” is effectively the name of that protocol (“serial-peripheral interface” is probably too long). (“Wire” was probably a mistake, as the protocol it uses is typically called “TWI” or “I2C”.)
    * Use camel case function names, not underscore. For example, **analogRead**, not **analog_read**. Or **myNewFunction**, not **my_new_function**. We've adopted this from Processing.org for readability's sake.
  5. The final suggestion was made by Arduino users van_der_decken and ubidefeo. "It is not recommended to perform lengthy operations within interrupts." "The proper way to use IRQs is generally by setting a flag that will be checked inside your loop." Putting serial.println() inside periodicCallback() doesn't seem like a wise decision, the code in the example can mislead someone like me who is unfamiliar with rtc.

I also reported this issue on the Arduino community forum, see https://forum.arduino.cc/t/there-may-be-some-conflict-between-rtc-and-the-serial/1169836.

Another related issue is: arduino/ArduinoCore-renesas#138

I hope the document can be improved. Thank you for reading this issue!

@karlsoderby
Copy link
Contributor

Hi @eMUQI . We have updated the RTC tutorial with regard to the feedback you left here, and using the example provided by Ubi in the forum thread. Thanks for reporting it in such detail.

PR for docs update: #1370

@eMUQI
Copy link
Author

eMUQI commented Oct 7, 2023

thanks!

@eMUQI eMUQI closed this as completed Oct 7, 2023
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

No branches or pull requests

2 participants