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 DIO/QIO incorrect SPI_CHECK #2738

Closed
michprev opened this issue Nov 24, 2018 · 2 comments
Closed

SPI DIO/QIO incorrect SPI_CHECK #2738

michprev opened this issue Nov 24, 2018 · 2 comments

Comments

@michprev
Copy link

Environment

  • IDF version: v3.1.1-46-gcdd3131f8, also present in 0d7f2d7

Problem Description

SPI_CHECK(!((trans_desc->flags & (SPI_TRANS_MODE_DIO|SPI_TRANS_MODE_QIO)) && (!(handle->cfg.flags & SPI_DEVICE_HALFDUPLEX))), "incompatible iface params", ESP_ERR_INVALID_ARG);

Shouldn't it be
SPI_CHECK(!((trans_desc->flags & (SPI_TRANS_MODE_DIO|SPI_TRANS_MODE_QIO)) && (handle->cfg.flags & SPI_DEVICE_HALFDUPLEX)), "incompatible iface params", ESP_ERR_INVALID_ARG);
?

@negativekelvin
Copy link
Contributor

No because the error is when halfduplex is not set which the bAND will give you 0 but you need 1 to complete the AND

@michprev
Copy link
Author

Yay, I thought that SPI_DEVICE_HALFDUPLEX represents standard 3-wire half duplex mode. Thank you.

0xFEEDC0DE64 pushed a commit to 0xFEEDC0DE64/esp-idf that referenced this issue May 5, 2021
* Other Arduino cores uses a macro to redefine libc abs() to take any
  type, meaning abs(-3.3) == 3.3 not the normal libc result of 3.

* 1e4bf14 (espressif#1783) replaced similar min, max macros with c++ stdlib. However
  this change includes <algorithm> after the line which defines the abs() macro.
  <algorithm> includes <cstdlib> which undefines abs() and re-defines it.

* This means abs() becomes the plain libc version again which only takes
  integers, so abs(-3.3) == 3. As reported here:
  espressif#3405

This fix tries to keep in the spirit of espressif#1783 by using libstdc++. The other
option would be to include <cstdlib> before defining the abs() macro, so it
doesn't get undef-ed again later on.
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

2 participants