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

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

Open
Tnthr opened this issue Aug 30, 2023 · 4 comments
Labels
external Cause is external to this library (does not mean a work around for it isn't a valid solution)

Comments

@Tnthr
Copy link

Tnthr commented Aug 30, 2023

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):

  • OS: Win10
  • Build Environment: Arduino IDE 2.2.0
  • Board target: Arduino Nano Every
  • Library version: 2.4.2

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
Copy link
Owner

Makuna commented Aug 30, 2023

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 Makuna added the external Cause is external to this library (does not mean a work around for it isn't a valid solution) label Aug 30, 2023
@Makuna Makuna changed the title RtcDS3231.h requestFrom() function with ambiguous declaration (Arduino Nano Every) RtcDS3231.h requestFrom() function with ambiguous declaration Aug 31, 2023
@Makuna
Copy link
Owner

Makuna commented Aug 31, 2023

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
Copy link
Author

Tnthr commented Sep 10, 2023

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
Copy link
Owner

Makuna commented Sep 10, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external Cause is external to this library (does not mean a work around for it isn't a valid solution)
Projects
None yet
Development

No branches or pull requests

2 participants