Skip to content

library: Wire/SPI: change default pins type #1438

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Jul 6, 2021

Conversation

fpistm
Copy link
Member

@fpistm fpistm commented Jul 6, 2021

Since _ALTx introduction, default Arduino API using uint8_t type for pin number could not use them and raised raised this error:

warning: unsigned conversion from 'int' to 'uint8_t' {aka 'unsigned char'} changes value from '260' to '4' [-Woverflow]
   91 | #define PB5_ALT1                (PB5  | ALT1)
      |                                 ~~~~~~^~~~~~~
sketch_jul06a.ino:10:20: note: in expansion of macro 'PB5_ALT1'
   10 |   SPIClass SPI_3rd(PB5_ALT1, PC11, PC10);
      |                    ^~~~~~~~

Moving API to uint32_t allows to use _ALTx, anyway some libraries used the default pin definition as an uint8_t from the variant definition, for example:

#ifndef PIN_SPI_MOSI
  #define PIN_SPI_MOSI          PB5_ALT1
#endif

Which is stored in:

static const uint32_t MISO = PIN_SPI_MISO;

With Arduino SD library:
https://github.com/arduino-libraries/SD/blob/a64c2bd907460dd01cef07fff003550cfcae0119/src/utility/Sd2Card.h#L75-L83

libraries\SD\src\utility\Sd2PinMap.h:28:28: warning: conversion from 'uint32_t' {aka 'long unsigned int'} to 'uint8_t' {aka 'unsigned char'} changes value from '260' to '4' [-Woverflow]
   28 |   uint8_t const MOSI_PIN = MOSI;
      |                            ^~~~

In this case this is not risky as those conversion will use the correct pin for SOFTWARE_SPI (GPIO toggling) not related to a peripheral and in major way default pins uses value without _ALTx which mean an unit8_t size and so does not raised warning.
Anyway not all libraries could use _ALTx feature.

Fixes #1432

fpistm added 2 commits July 6, 2021 10:38
This allows _ALTx pins usage.

Fixes stm32duino#1432

Signed-off-by: Frederic Pillon <frederic.pillon@st.com>
This allows _ALTx pins usage.

Signed-off-by: Frederic Pillon <frederic.pillon@st.com>
@fpistm fpistm added enhancement New feature or request arduino compatibility labels Jul 6, 2021
@fpistm fpistm added this to the 2.x.x milestone Jul 6, 2021
Copy link
Contributor

@ABOSTM ABOSTM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fpistm fpistm merged commit 26340ec into stm32duino:master Jul 6, 2021
@fpistm fpistm deleted the library_API branch July 6, 2021 15:17
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SPI library: can't accept pin numbers with _ALTx in constructor
2 participants