stickbreaker / arduino-esp32

Arduino core for the ESP32
38 stars 23 forks source link

Can't build: implicit declaration of function 'dumpCmdQueue' in esp-hal-i2c #13

Closed dstoiko closed 6 years ago

dstoiko commented 6 years ago

Hardware:

Board: ESP32 Dev Module (DevKit-C) Core Installation/update date: latest IDE name: IDF component Flash Frequency: 40MHz Upload Speed: 115200

Description:

I get the following error when trying to build using your repo as my arduino-esp32 submodule in my arduino-as-idf-component configuration:

./components/arduino/cores/esp32/esp32-hal-i2c.c: In function 'fillCmdQueue':
./components/arduino/cores/esp32/esp32-hal-i2c.c:511:11: error: implicit declaration of function 'dumpCmdQueue' [-Werror=implicit-function-declaration]
           dumpCmdQueue(i2c);

Any idea where that might come from?

stickbreaker commented 6 years ago

@dstoiko my bad

I need to move void IRAM_ATTR dumpCmdQueue(i2c_t *i2c){} higher in esp32-hal-i2c.c

move it before static void IRAM_ATTR fillCmdQueue(i2c_t * i2c, bool INTS){}

Or add void IRAM_ATTR dumpCmdQueue(i2c_t *i2c); to the header file. It would be better to move it in the .c

Chuck.

me-no-dev commented 6 years ago

yup ;) missing forward declaration (put on top of the C file, not the header)

stickbreaker commented 6 years ago

@me-no-dev You bleary Eyed yet from starring at my code?

Chuck.

me-no-dev commented 6 years ago

:D almost yes :D had to do lots of fixing of the style to even become readable ;) Still going through it all (lots of things to take care of in the new country and office). Got it to compile cleanly though and will see to bring that last commit you just pushed into my copy (pr/XXX branch of the main repo)

stickbreaker commented 6 years ago

@me-no-dev You can probably see that 'c' is not my native language. I like Pascal. c's stoopid bracies alignment makes no sense to me.

if()
  {
}
else 
  {
}

or is it this way

if()
{
  }
else 
{
  }

I can read this easier

if() {
  }
else {
  }

Chuck.

me-no-dev commented 6 years ago
if () {

} else {

}

This I can read :D and is the style that is generally used throughout IDF and therefore Arduino ;) code needs to be structured properly to be merged, but I get that you have a different preference, so I opted to work on it on the side and not mess with your styling for now :P

stickbreaker commented 6 years ago

@me-no-dev The my current code is a hack. I more ideas on how It could be better, but, it is working now. Are you thinking of shoving a prettified version into the main distro?

My future plans are to spend sometime on slave mode. and 10bit between ESP32's and Arduino's I can make a UNO do just about anything on the I2C bus.

I have an idea matriculating between my ears to make procQueue() evaporate. Either have the addQueue() be the jump off, or setup a thread feed by a FreeRTOS queue. From a programmers standpoint, I want it as simple as possible. None of this forced march order that exists now. But there are a couple of encapsulation/ordering issues that may crop up with multiple thread access.

Chuck.

me-no-dev commented 6 years ago

I see more things that could maybe be improved :) we will get this working @stickbreaker ! Good job so far! Do not worry at all ;)

stickbreaker commented 6 years ago

@me-no-dev look at the base README.md on my Repo. Would something like that be possible? If that readme contained links to all of the libraries documentation it would be a good jumping off point. And, aren't there automated ways to convert from .md format to html.

Chuck.

stickbreaker commented 6 years ago

@me-no-dev Point out the problems, I'm not shy. I can take criticism. And, of course, I can Ignore you! 😀 You're not my Boss!

Chuck.

dstoiko commented 6 years ago

Thanks @stickbreaker, did the trick indeed. There were a few more warnings-turned-errors, with the Wire library, different declaration / initialization orders between Wire.h and Wire.cpp.

Basically changed TwoWire class in Wire.cpp to:

TwoWire::TwoWire(uint8_t bus_num)
    : num(bus_num & 1),
      sda(-1),
      scl(-1),
      i2c(NULL),
      rxIndex(0),
      rxLength(0),
      rxQueued(0),
      txIndex(0),
      txLength(0),
      txAddress(0),
      txQueued(0),
      transmitting(0),
      _timeOutMillis(50),
      last_error(I2C_ERROR_OK),
      _dump(false) {
}

...and in Wire.h:

class TwoWire : public Stream {
   protected:
    uint8_t num;
    int8_t sda;
    int8_t scl;
    i2c_t* i2c;

    uint8_t rxBuffer[I2C_BUFFER_LENGTH];
    uint16_t rxIndex;
    uint16_t rxLength;
    uint16_t rxQueued;  //@stickBreaker

    uint8_t txBuffer[I2C_BUFFER_LENGTH];
    uint16_t txIndex;
    uint16_t txLength;
    uint16_t txAddress;
    uint16_t txQueued;  //@stickbreaker

    uint8_t transmitting;
    /* slave Mode, not yet Stickbreaker
                    static user_onRequest uReq[2];
                    static user_onReceive uRcv[2];
        void onRequestService(void);
        void onReceiveService(uint8_t*, int);
    */
    uint16_t _timeOutMillis;
    i2c_err_t last_error;  // @stickBreaker from esp32-hal-i2c.h
    i2c_err_t processQueue(uint32_t* readCount);
    bool _dump;

    ...
}
dstoiko commented 6 years ago

Do you know in which particular test cases the i2c bus keeps throwing that Busy timeout error? Seemed to happen pretty randomly for me, would like to reproduce the error and hopefully locate it. Works fine now with your version of the i2c HAL + Wire lib, but I don't know why...

Thanks a lot!

stickbreaker commented 6 years ago

The original problem I found was how the sendStop=false was being handled. The exists I2C driver was using the the hardware's Command Buffer Continuation Command (END) as a pause. But, the Hardware StateMachine has a freerunning timer that would expire in 13.1ms if the Command Buffer was not continued. The FreeRTOS scheduler would do a task switch, or the Application programmer would assume that a beginTransmission() -> write() -> endTransmission(false); was a complete I2C operation.

A valid I2C transmission is a START -> at least one data byte -> STOP. The ReSTART (sendStop=FALSE) is just another START thrown into this sequence: START -> at least one Data Byte -> ReSTART -> at least one data byte -> STOP.

If you are still getting a Busy Timeout, there are only three possible circumstances that can generate a bus_busy with my driver, First is that the SLAVE device is stretching SCL to Pause the MASTER(esp32). @lonerzzz has a device that stretches SCL for longer than the timeout (13.1ms), we are trying to accommodate this activity. I sent him piece of code to try. I haven't heard back yet. The second is that another BUS MASTER is using the bus incorrectly, starting a transaction, but never issuing the STOP. The Timeout will occur if the SCL clock does not cycle every 13.1ms. The Third method is if there is noise on the BUS and the ESP32 interprets this noise as a valid START. (which starts the timeout counter).

Chuck.

stickbreaker commented 6 years ago

@dstoiko Just pushed your recommended changes to clear the compiler warnings. I'm closing this issue.

dstoiko commented 6 years ago

Thanks @stickbreaker!