stevemarple / SoftWire

Software I2C implementation for Arduino and other Wiring-type environments
GNU Lesser General Public License v2.1
136 stars 31 forks source link

Modification for Arduino Core STM32 #28

Closed technik-gegg closed 2 years ago

technik-gegg commented 2 years ago

On the Arduino Core STM32 framework pin definitions have moved from uint8_t to uint32_t in order to reflect higher pin numbers on some of the newer MCUs available. Using uint8_t for storing pin numbers internally may cause cut offs and thus, such pins will not be adressed and function correctly.

Hence I've introduced a new typedef "pin_t" which will be set to uint32_t on the Adruino Core STM32 but stays as it is (uint8_t) on other frameworks:

#if !defined(STM32_CORE_VERSION)
  typedef uint8_t     pin_t;
#else
  typedef uint32_t    pin_t;
#endif

I've also added a workaround for the missing ATOMIC_BLOCK in non AVR architectures, I've found on Stackoverflow.

stevemarple commented 2 years ago

This is a useful set of changes but as there are two different changesets here:

  1. Fixing the pin type
  2. Fixing the missing ATOMIC_BLOCK operation

I'd like to see a separate pull request for each.

For the pin type you mentioned that only the newer STM32 frameworks use uint32_t. Rather than assuming a specific type is it possible to automatically use the same type as the framework does, probably by using decltype ? Maybe something like this would work?

typedef decltype(some_pin_variable) pin_t;

Ideally this would be used for all Arduino cores, not just STM32, but I don't think there is a consistent way this can be applied. AVR cores (older ones anyway) use #define for some pins so restricting this change to within a #ifdef block is safest.

technik-gegg commented 2 years ago

This is a useful set of changes but as there are two different changesets here:

  1. Fixing the pin type
  2. Fixing the missing ATOMIC_BLOCK operation

I'd like to see a separate pull request for each.

Ok, removed the ATOMIC_BLOCK changes from this PR.

For the pin type you mentioned that only the newer STM32 frameworks use uint32_t. Rather than assuming a specific type is it possible to automatically use the same type as the framework does, probably by using decltype ? Maybe something like this would work?

typedef decltype(some_pin_variable) pin_t;

Ideally this would be used for all Arduino cores, not just STM32, but I don't think there is a consistent way this can be applied. AVR cores (older ones anyway) use #define for some pins so restricting this change to within a #ifdef block is safest.

I haven't found anything in Arduino.h that would qualify for this notation.

stevemarple commented 2 years ago

I've locally taken all of these commits, squashed them into one and compared to master. There are still changes to ATOMIC_BLOCK/CRITICAL section. I want a single changeset (and ideally just one commit, so I can see exactly what the change is) that adds the pin_t typedef.

I want to see the ATOMIC_BLOCK in a separate pull-request as this fixes a separate issue.