Fix ambiguous for TwoWire::requestFrom() methods and align API with Arduino.cc (#8817)

safocl · lucasssvaz · P-R-O-C-H-Y · web-flow · commit 7966f4ae7966 · 2024-01-31T12:50:35.000+02:00
* Fix ambiguous for TwoWire::requestFrom() methods. * Remove TwoWire::begin(int) overload Inside the function, this overload truncated the data type to a shorter one. This could break some users' hopes. * Rewrite TwoWire with using HardwareI2C - implement proposal #8818 (comment) to bring the HARDWARE interface into compliance * Fix TwoWire::end() return type. * Fix TwoWire::setClock() return type. * Fix no return statement in the TwoWire::requestFrom. * fix(libraries/Wire): fix bad return-statement Remove non-void values of the return-statements in function returning 'void'. * style(libraries/Wire): replace tabs with spaces * refactor(libraries/Wire): use slave without support TwoWire::begin(uint8_t address) should be available without slave support by SoC? * refactor(libraries/Wire): remove unused variables Compiler reports "Wire.cpp:393:15: error: variable 'err' set but not used [-Werror=unused-but-set-variable]". * refactor(libraries/Wire): remove unused variables Compiler reports "Wire.cpp:337:15: error: variable 'err' set but not used [-Werror=unused-but-set-variable]". * fix(libraries/Wire): hide slave support elements TwoWire::user_onRequest is used only in conjunction with slave support? * refactor(libraries/Wire): remove temporary comment * fix(libraries/Wire): restore an accidentally deleted implementation TwoWire::endTransmission() was accidentally deleted from a cpp file. * refactor(libraries/Wire): return return types In TwoWire class return return types. * fix(libraries/Wire): fix return type * refactor(libraries/Wire): add return statement if slave isn't supported Co-authored-by: Jan Procházka <90197375+P-R-O-C-H-Y@users.noreply.github.com> * refactor(libraries/Wire): fix indentation Co-authored-by: Jan Procházka <90197375+P-R-O-C-H-Y@users.noreply.github.com> * refactor(libraries/Wire): fix indentation Co-authored-by: Jan Procházka <90197375+P-R-O-C-H-Y@users.noreply.github.com> * refactor(libraries/Wire): fix indentation Co-authored-by: Jan Procházka <90197375+P-R-O-C-H-Y@users.noreply.github.com> * refactor(libraries/Wire): remove unnecessary empty lines --------- Co-authored-by: Lucas Saavedra Vaz <32426024+lucasssvaz@users.noreply.github.com> Co-authored-by: Jan Procházka <90197375+P-R-O-C-H-Y@users.noreply.github.com>
diff --git a/cores/esp32/HardwareI2C.h b/cores/esp32/HardwareI2C.h
@@ -0,0 +1,42 @@
+/*
+  Copyright (c) 2016 Arduino LLC.  All right reserved.
+
+  This library is free software; you can redistribute it and/or
+  modify it under the terms of the GNU Lesser General Public
+  License as published by the Free Software Foundation; either
+  version 2.1 of the License, or (at your option) any later version.
+
+  This library is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+  See the GNU Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public
+  License along with this library; if not, write to the Free Software
+  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+*/
+
+#pragma once
+
+#include <inttypes.h>
+#include "Stream.h"
+
+class HardwareI2C : public Stream
+{
+  public:
+    virtual bool begin() = 0;
+    virtual bool begin(uint8_t address) = 0;
+    virtual bool end() = 0;
+
+    virtual bool setClock(uint32_t freq) = 0;
+  
+    virtual void beginTransmission(uint8_t address) = 0;
+    virtual uint8_t endTransmission(bool stopBit) = 0;
+    virtual uint8_t endTransmission(void) = 0;
+
+    virtual size_t requestFrom(uint8_t address, size_t len, bool stopBit) = 0;
+    virtual size_t requestFrom(uint8_t address, size_t len) = 0;
+
+    virtual void onReceive(void(*)(int)) = 0;
+    virtual void onRequest(void(*)(void)) = 0;
+};
diff --git a/libraries/Wire/src/Wire.cpp b/libraries/Wire/src/Wire.cpp
@@ -149,7 +149,7 @@ bool TwoWire::setPins(int sdaPin, int sclPin)
     return !i2cIsInit(num);
 }
 
-bool TwoWire::allocateWireBuffer(void)
+bool TwoWire::allocateWireBuffer()
 {
     // or both buffer can be allocated or none will be
     if (rxBuffer == NULL) {
@@ -171,7 +171,7 @@ bool TwoWire::allocateWireBuffer(void)
     return true;
 }
 
-void TwoWire::freeWireBuffer(void)
+void TwoWire::freeWireBuffer()
 {
     if (rxBuffer != NULL) {
         free(rxBuffer);
@@ -424,7 +424,7 @@ uint16_t TwoWire::getTimeOut()
     return _timeOutMillis;
 }
 
-void TwoWire::beginTransmission(uint16_t address)
+void TwoWire::beginTransmission(uint8_t address)
 {
 #if SOC_I2C_SUPPORT_SLAVE
     if(is_slave){
@@ -492,7 +492,12 @@ uint8_t TwoWire::endTransmission(bool sendStop)
     return 4;
 }
 
-size_t TwoWire::requestFrom(uint16_t address, size_t size, bool sendStop)
+uint8_t TwoWire::endTransmission()
+{
+    return endTransmission(true);
+}
+
+size_t TwoWire::requestFrom(uint8_t address, size_t size, bool sendStop)
 {
 #if SOC_I2C_SUPPORT_SLAVE
     if(is_slave){
@@ -550,6 +555,10 @@ size_t TwoWire::requestFrom(uint16_t address, size_t size, bool sendStop)
     return rxLength;
 }
 
+size_t TwoWire::requestFrom(uint8_t address, size_t size){
+    return requestFrom(address, size, true);
+}
+
 size_t TwoWire::write(uint8_t data)
 {
     if (txBuffer == NULL){
@@ -574,13 +583,13 @@ size_t TwoWire::write(const uint8_t *data, size_t quantity)
 
 }
 
-int TwoWire::available(void)
+int TwoWire::available()
 {
     int result = rxLength - rxIndex;
     return result;
 }
 
-int TwoWire::read(void)
+int TwoWire::read()
 {
     int value = -1;
     if (rxBuffer == NULL){
@@ -593,7 +602,7 @@ int TwoWire::read(void)
     return value;
 }
 
-int TwoWire::peek(void)
+int TwoWire::peek()
 {
     int value = -1;
     if (rxBuffer == NULL){
@@ -606,70 +615,27 @@ int TwoWire::peek(void)
     return value;
 }
 
-void TwoWire::flush(void)
+void TwoWire::flush()
 {
     rxIndex = 0;
     rxLength = 0;
     txLength = 0;
     //i2cFlush(num); // cleanup
 }
 
-size_t TwoWire::requestFrom(uint8_t address, size_t len, bool sendStop)
-{
-    return requestFrom(static_cast<uint16_t>(address), static_cast<size_t>(len), static_cast<bool>(sendStop));
-}
-  
-uint8_t TwoWire::requestFrom(uint8_t address, uint8_t len, uint8_t sendStop)
-{
-    return requestFrom(static_cast<uint16_t>(address), static_cast<size_t>(len), static_cast<bool>(sendStop));
-}
-
-uint8_t TwoWire::requestFrom(uint16_t address, uint8_t len, uint8_t sendStop)
-{
-    return requestFrom(address, static_cast<size_t>(len), static_cast<bool>(sendStop));
-}
-
-/* Added to match the Arduino function definition: https://github.com/arduino/ArduinoCore-API/blob/173e8eadced2ad32eeb93bcbd5c49f8d6a055ea6/api/HardwareI2C.h#L39
- * See: https://github.com/arduino-libraries/ArduinoECCX08/issues/25
-*/
-uint8_t TwoWire::requestFrom(uint16_t address, uint8_t len, bool stopBit)
-{
-    return requestFrom((uint16_t)address, (size_t)len, stopBit);
-}
-
-uint8_t TwoWire::requestFrom(uint8_t address, uint8_t len)
-{
-    return requestFrom(static_cast<uint16_t>(address), static_cast<size_t>(len), true);
-}
-
-uint8_t TwoWire::requestFrom(uint16_t address, uint8_t len)
-{
-    return requestFrom(address, static_cast<size_t>(len), true);
-}
-
-uint8_t TwoWire::requestFrom(int address, int len)
-{
-    return requestFrom(static_cast<uint16_t>(address), static_cast<size_t>(len), true);
-}
-
-uint8_t TwoWire::requestFrom(int address, int len, int sendStop)
-{
-    return static_cast<uint8_t>(requestFrom(static_cast<uint16_t>(address), static_cast<size_t>(len), static_cast<bool>(sendStop)));
-}
-
-void TwoWire::beginTransmission(int address)
-{
-    beginTransmission(static_cast<uint16_t>(address));
-}
-
-void TwoWire::beginTransmission(uint8_t address)
+void TwoWire::onReceive( void (*function)(int) )
 {
-    beginTransmission(static_cast<uint16_t>(address));
+#if SOC_I2C_SUPPORT_SLAVE
+    user_onReceive = function;
+#endif
 }
 
-uint8_t TwoWire::endTransmission(void)
+// sets function called on slave read
+void TwoWire::onRequest( void (*function)(void) )
 {
-    return endTransmission(true);
+#if SOC_I2C_SUPPORT_SLAVE
+    user_onRequest = function;
+#endif
 }
 
 #if SOC_I2C_SUPPORT_SLAVE
@@ -714,17 +680,6 @@ void TwoWire::onRequestService(uint8_t num, void * arg)
     }
 }
 
-void TwoWire::onReceive( void (*function)(int) )
-{
-  user_onReceive = function;
-}
-
-// sets function called on slave read
-void TwoWire::onRequest( void (*function)(void) )
-{
-  user_onRequest = function;
-}
-
 #endif /* SOC_I2C_SUPPORT_SLAVE */
 
 TwoWire Wire = TwoWire(0);
diff --git a/libraries/Wire/src/Wire.h b/libraries/Wire/src/Wire.h
@@ -30,11 +30,13 @@
 #if SOC_I2C_SUPPORTED
 
 #include <esp32-hal.h>
+#include <esp32-hal-log.h>
 #if !CONFIG_DISABLE_HAL_LOCKS
 #include "freertos/FreeRTOS.h"
 #include "freertos/task.h"
 #include "freertos/semphr.h"
 #endif
+#include "HardwareI2C.h"
 #include "Stream.h"
 
 // WIRE_HAS_BUFFER_SIZE means Wire has setBufferSize()
@@ -50,7 +52,7 @@ typedef void(*user_onRequest)(void);
 typedef void(*user_onReceive)(uint8_t*, int);
 #endif /* SOC_I2C_SUPPORT_SLAVE */
 
-class TwoWire: public Stream
+class TwoWire: public HardwareI2C
 {
 protected:
     uint8_t num;
@@ -81,12 +83,41 @@ class TwoWire: public Stream
     static void onReceiveService(uint8_t, uint8_t*, size_t, bool, void *);
 #endif /* SOC_I2C_SUPPORT_SLAVE */
     bool initPins(int sdaPin, int sclPin);
-    bool allocateWireBuffer(void);
-    void freeWireBuffer(void);
+    bool allocateWireBuffer();
+    void freeWireBuffer();
 
 public:
     TwoWire(uint8_t bus_num);
     ~TwoWire();
+
+    bool begin() override final
+    {
+        return begin(-1, -1);
+    }
+
+    bool begin(uint8_t address) override final
+    {
+#if SOC_I2C_SUPPORT_SLAVE
+        return begin(address, -1, -1, 0);
+#else
+        log_e("I2C slave is not supported on " CONFIG_IDF_TARGET);
+        return false;
+#endif
+    }
+
+    bool end() override;
+
+    bool setClock(uint32_t freq) override;
+  
+    void beginTransmission(uint8_t address) override;
+    uint8_t endTransmission(bool stopBit) override;
+    uint8_t endTransmission() override;
+
+    size_t requestFrom(uint8_t address, size_t len, bool stopBit) override;
+    size_t requestFrom(uint8_t address, size_t len) override;
+
+    void onReceive(void(*)(int)) override;
+    void onRequest(void(*)(void)) override;
     
     //call setPins() first, so that begin() can be called without arguments from libraries
     bool setPins(int sda, int scl);
@@ -95,78 +126,22 @@ class TwoWire: public Stream
 #if SOC_I2C_SUPPORT_SLAVE
     bool begin(uint8_t slaveAddr, int sda, int scl, uint32_t frequency);
 #endif /* SOC_I2C_SUPPORT_SLAVE */
-    // Explicit Overload for Arduino MainStream API compatibility
-    inline bool begin()
-    {
-        return begin(-1, -1, static_cast<uint32_t>(0));
-    }
-#if SOC_I2C_SUPPORT_SLAVE
-    inline bool begin(uint8_t addr)
-    {
-        return begin(addr, -1, -1, 0);
-    }
-    inline bool begin(int addr)
-    {
-        return begin(static_cast<uint8_t>(addr), -1, -1, 0);
-    }
-#endif /* SOC_I2C_SUPPORT_SLAVE */
-    bool end();
 
     size_t setBufferSize(size_t bSize);
 
     void setTimeOut(uint16_t timeOutMillis); // default timeout of i2c transactions is 50ms
     uint16_t getTimeOut();
 
-    bool setClock(uint32_t);
     uint32_t getClock();
 
-    void beginTransmission(uint16_t address);
-    void beginTransmission(uint8_t address);
-    void beginTransmission(int address);
-
-    uint8_t endTransmission(bool sendStop);
-    uint8_t endTransmission(void);
-
-    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);
-
-    size_t write(uint8_t);
-    size_t write(const uint8_t *, size_t);
-    int available(void);
-    int read(void);
-    int peek(void);
-    void flush(void);
-
-    inline size_t write(const char * s)
-    {
-        return write((uint8_t*) s, strlen(s));
-    }
-    inline size_t write(unsigned long n)
-    {
-        return write((uint8_t)n);
-    }
-    inline size_t write(long n)
-    {
-        return write((uint8_t)n);
-    }
-    inline size_t write(unsigned int n)
-    {
-        return write((uint8_t)n);
-    }
-    inline size_t write(int n)
-    {
-        return write((uint8_t)n);
-    }
+    size_t write(uint8_t) override;
+    size_t write(const uint8_t *, size_t) override;
+    int available() override;
+    int read() override;
+    int peek() override;
+    void flush() override;
+
 #if SOC_I2C_SUPPORT_SLAVE
-    void onReceive( void (*)(int) );
-    void onRequest( void (*)(void) );
     size_t slaveWrite(const uint8_t *, size_t);
 #endif /* SOC_I2C_SUPPORT_SLAVE */
 };
