Makuna / Rtc

Arduino Library for RTCs, Ds1302, Ds1307, Ds3231, Ds3232, Ds3234 and Pcf8563/BM8563 with deep support. Please refer to the Wiki for more details. Please use the Github Discussions to ask questions as the GitHub Issues feature is used for bug tracking.
GNU Lesser General Public License v3.0
369 stars 127 forks source link

(Arduino Nano Every) RtcDS3231.h requestFrom() function with ambiguous declaration #196

Open Tnthr opened 1 year ago

Tnthr commented 1 year ago

Describe the bug IsDateTimeValid() causes a warning about ambiguous declaration. See below for error output. This can be seen with the DS3231_Simple.ino example script.

In file included from C:\Users\brian\AppData\Local\Temp\.arduinoIDE-unsaved2023730-15568-u5zvlf.mrk2s\DS3231_Simple\DS3231_Simple.ino:18:0:
c:\Users\brian\Source\libraries\Rtc_by_Makuna\src/RtcDS3231.h: In instantiation of 'uint8_t RtcDS3231<T_WIRE_METHOD>::getReg(uint8_t) [with T_WIRE_METHOD = TwoWire; uint8_t = unsigned char]':
c:\Users\brian\Source\libraries\Rtc_by_Makuna\src/RtcDS3231.h:272:32:   required from 'bool RtcDS3231<T_WIRE_METHOD>::IsDateTimeValid() [with T_WIRE_METHOD = TwoWire]'
C:\Users\brian\AppData\Local\Temp\.arduinoIDE-unsaved2023730-15568-u5zvlf.mrk2s\DS3231_Simple\DS3231_Simple.ino:87:30:   required from here
c:\Users\brian\Source\libraries\Rtc_by_Makuna\src/RtcDS3231.h:663:16: warning: ISO C++ says that these are ambiguous, even though the worst conversion for the first is better than the worst conversion for the second:
         size_t bytesRead = _wire.requestFrom(DS3231_ADDRESS, (uint8_t)1);
                ^~~~~~~~~
In file included from C:\Users\brian\AppData\Local\Temp\.arduinoIDE-unsaved2023730-15568-u5zvlf.mrk2s\DS3231_Simple\DS3231_Simple.ino:17:0:
C:\Users\brian\AppData\Local\Arduino15\packages\arduino\hardware\megaavr\1.8.8\libraries\Wire\src/Wire.h:63:12: note: candidate 1: size_t TwoWire::requestFrom(int, int)
     size_t requestFrom(int, int);
            ^~~~~~~~~~~
C:\Users\brian\AppData\Local\Arduino15\packages\arduino\hardware\megaavr\1.8.8\libraries\Wire\src/Wire.h:61:12: note: candidate 2: virtual size_t TwoWire::requestFrom(uint8_t, size_t)
     size_t requestFrom(uint8_t, size_t);
            ^~~~~~~~~~~

To Reproduce Steps to reproduce the behavior:

  1. compile DS3231_Simple.ino

Expected behavior No warnings during compile.

Development environment (please complete the following information):

Minimal Sketch that reproduced the problem: DS3231_Simple.ino

Additional context Suggested fix: Change the type cast to size_t on line 663 of RtcDS3231.h to match the "candidate 2" form of requestFrom().

        // control register
+++     size_t bytesRead = _wire.requestFrom(DS3231_ADDRESS, (size_t)1);
---     size_t bytesRead = _wire.requestFrom(DS3231_ADDRESS, (uint8_t)1);
        if (1 != bytesRead)
        {
Makuna commented 1 year ago

This "issue" is caused by the Wire library for that platform not following the standards. They are missing the one that takes two arguments, both uint8_t.

From AVR Wire.h (and many other platforms)

    uint8_t requestFrom(uint8_t, uint8_t);
    uint8_t requestFrom(uint8_t, uint8_t, uint8_t);
    uint8_t requestFrom(uint8_t, uint8_t, uint32_t, uint8_t, uint8_t);
    uint8_t requestFrom(int, int);
    uint8_t requestFrom(int, int, int);

from the nano every Wire.h

    size_t requestFrom(uint8_t, size_t);
    size_t requestFrom(uint8_t, size_t, bool);
    size_t requestFrom(int, int);
    size_t requestFrom(int, int, int);

and from ESP32

    size_t requestFrom(uint16_t address, size_t size, bool sendStop);
    uint8_t requestFrom(uint16_t address, uint8_t size, bool sendStop);
    uint8_t requestFrom(uint16_t address, uint8_t size, uint8_t sendStop);
    size_t requestFrom(uint8_t address, size_t len, bool stopBit);
    uint8_t requestFrom(uint16_t address, uint8_t size);
    uint8_t requestFrom(uint8_t address, uint8_t size, uint8_t sendStop);
    uint8_t requestFrom(uint8_t address, uint8_t size);
    uint8_t requestFrom(int address, int size, int sendStop);
    uint8_t requestFrom(int address, int size);

The library is written to work on minimal used standard and does compile without warnings if the platform is written to those standards.

Makuna commented 1 year ago

I would suggest going to the Nano Every issue tracking and add an issue for them to supply a compliant method that meets the standards set already on other platforms.

Tnthr commented 1 year ago

After some searching around this is what I could find. The Nano Every is a megaAVR board which is already using the ArduinoCore-API, while AVR based boards are not yet apparently. My assumption is that the AVR boards will eventually move to the standardized API as well although I didn't really dig into that. In the API the definitions of the requestFrom() function use a uint8_t and size_t type variables (linked and shown below). So I think the most compatible way to use the requestFrom function would be with those types.

I'm no expert though so please help me out if I'm missing something here.

https://github.com/arduino/ArduinoCore-API/blob/master/api/HardwareI2C.h

    virtual size_t requestFrom(uint8_t address, size_t len, bool stopBit) = 0;
    virtual size_t requestFrom(uint8_t address, size_t len) = 0;
Makuna commented 1 year ago

THE AVR platforms set the standards for Arduino. They were the first and only type of Arduino you could get for many years. If they change it, they will break sketches and libraries unless they add the mapping call.

Currently today, AVR doesn't support that one; and neither does some other platforms, so right now the most common and the longest supported one is the one I am using.