Skip to content

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

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

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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