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

SPI.transfer(buffer, size) is missing #1623

Closed
PaulStoffregen opened this issue Jul 11, 2018 · 13 comments
Closed

SPI.transfer(buffer, size) is missing #1623

PaulStoffregen opened this issue Jul 11, 2018 · 13 comments

Comments

@PaulStoffregen
Copy link

ESP32's SPI library is missing overloaded function SPI.transfer(buffer, size).

Here is Arduino's documentation.

https://www.arduino.cc/en/Reference/SPITransfer

Arduino sketches and libraries using this function will not compile for ESP32. This is a very easy fix, just add this line in SPI.h

  void transfer(uint8_t * data, uint32_t size) { transferBytes(data, data, size); }
@me-no-dev
Copy link
Member

why not make a PR 😄

@me-no-dev
Copy link
Member

don't implement it in the header like that though

@PaulStoffregen
Copy link
Author

I released Arduino Ethernet 2.0.0 today.
https://github.com/arduino-libraries/Ethernet/releases/tag/2.0.0

It requires this issue fixed to compile for ESP32. Users can of course still use the old versions, or Adafruit's Ethernet2 or others... without the improvement performance and new features.

@iafilius
Copy link
Contributor

iafilius commented Dec 2, 2018

Hi, just a "me too" message (for libaries 1.0.0 and 1.0.1-rc1). The quick fix by adding the oneliner to SPI.h worked for me.

iafilius added a commit to iafilius/arduino-esp32 that referenced this issue Dec 2, 2018
Splitted suggested fix issue espressif#1623 in a header and source part. Dit not completely dive into the code.
Giving data twice as parameter feels wrong, but it compiles, and i can succesfully use the W5500 with SPI with this fix.
Doesn't compile without.
iafilius added a commit to iafilius/arduino-esp32 that referenced this issue Dec 2, 2018
Splitted suggested fix issue espressif#1623 in a header and source part. Dit not completely dive into the code.
Giving data twice as parameter feels wrong, but it compiles, and i can succesfully use the W5500 with SPI with this fix.
Doesn't compile without.
@iafilius
Copy link
Contributor

iafilius commented Dec 2, 2018

Hi, created pull request #2136 for it. I hope the fix makes it into 1.0.1-rc2. Regards

@PaulStoffregen
Copy link
Author

While not strictly necessary to get Ethernet to work, you will see slightly improved performance on the W5500 chip if you also add SPI.transfer(const void *txBuffer, void *rxBuffer, size_t count) and define SPI_HAS_TRANSFER_BUF.

Here's where it's actually used by the Ethernet library:
https://github.com/arduino-libraries/Ethernet/blob/master/src/utility/w5100.cpp#L372

If SPI.h does not have this function, Ethernet will revert to using single byte transfers.

me-no-dev pushed a commit that referenced this issue Dec 2, 2018
…2136)

* #1623, implementing suggested change

Splitted suggested fix issue #1623 in a header and source part. Dit not completely dive into the code.
Giving data twice as parameter feels wrong, but it compiles, and i can succesfully use the W5500 with SPI with this fix.
Doesn't compile without.

* #1623, implementing suggested change SPI.h/cpp

Splitted suggested fix issue #1623 in a header and source part. Dit not completely dive into the code.
Giving data twice as parameter feels wrong, but it compiles, and i can succesfully use the W5500 with SPI with this fix.
Doesn't compile without.
@iafilius
Copy link
Contributor

iafilius commented Dec 2, 2018

@me-no-dev Thanks for accepting accepting the commit

@PaulStoffregen ,

you mean something like:

SPI.h:
class SPIClass
<>
public:
<>
void transfer(const void * txBuffer, const void * rxBuffer, uint32_t size);
<>

SPI.cpp:

// SPI_HAS_TRANSFER_BUF - is defined to signify that this library supports
// a version of transfer which allows you to pass in both TX and RX buffer
// pointers, either of which could be NULL
#define SPI_HAS_TRANSFER_BUF 1
SPI.transfer(const void *txBuffer, void *rxBuffer, size_t count)
{
transferBytes(txBuffer, rxBuffer, size);
}

If i see it correctly, no corresponding transferBytes() is available with the right types (void*), and no void* type parameter functions are available yet. What do you have in mind?
Regards

@PaulStoffregen
Copy link
Author

Yes, about like that. The pointers are void, so not necessarily aligned to 32 bit boundaries (was an issue on ESP8266).

@Popcron07
Copy link

Hi, I ran into a problem which I think might have something to do with your changes.
When I try to compile my code I get this error message:

C:...\packages\esp32\hardware\esp32\1.0.0\libraries\SPI\src/SPI.h:69:13: note: candidate: uint8_t SPIClass::transfer(uint8_t)

 uint8_t transfer(uint8_t data);

         ^

C:...\packages\esp32\hardware\esp32\1.0.0\libraries\SPI\src/SPI.h:69:13: note: candidate expects 1 argument, 2 provided

@iafilius wrote it is strange to give the data twice and I think that is why I am not able to compile.
So please tell me, is this the cause for my error or do I have to search for something else?

Also I think this fellow might have the same problem and it really fits because his post is just a week old, too:
https://stackoverflow.com/questions/53795074/arduino-exit-status-1

Regards, Popcorn

@iafilius
Copy link
Contributor

Hi @Popcron07 , with not much info to work on, I see you're Using 1.0.0 libary. The Arduino dev/prerelease version i used was 1.0.1-rc1 prerelease, but the proposed fix of missing "SPI.transfer(buffer, size) is missing #1623" was pulled afterwards. This is the issue you seem to have. (As I did)

I case you're interested I wrote some demo code for using the W5500 and WiFi and using NTP as sample ethernet traffic.
You can find it on:
https://github.com/iafilius/ESP32_W5500_Ethernet_NTP_deepsleep_demo
It needs the fix, proposed by @PaulStoffregen, which i just created the pull request based on. .

So you or simply Apply PaulStofregen's patch, or my version into current libary and you're probably good to go. I see current prerelease 1.0.1-rc2 is available and included this fix. It is listen in the changelog as:
b5f3170 Fixed Arduino SPI/Ethernet compile issue as described in issue #1623 (#2136)

Does this help you?

Regards,
Arjan

@PaulStoffregen
Copy link
Author

Just to make sure we have a simple & clear description of how to reproduce this problem.

In Arduino 1.8.8, click Tools > Manage Libraries. Search for "Ethernet" and make sure version 2.0.0 is installed. Arduino 1.8.8 has Ethernet 2.0.0 by default, so this step is just to make sure you don't have an old version of Ethernet lingering in a location like Documents/Arduino/libraries.

Then open any of the examples, by clicking File > Examples > Ethernet.

Click Verify to reproduce the compile error. Easy, right?

@me-no-dev
Copy link
Member

Since the original topic is resolved, I am closing this issue. Feel free to continue commenting though :)

@ankurbashir
Copy link

Hi i am working on W5500 with ESP32 but got the error as mentioned below.

Initialize Ethernet with DHCP:
Failed to configure Ethernet using DHCP
Ethernet shield was not found. Sorry, can't run without hardware. :(

What i am missing here?

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

5 participants