stm32duino / Arduino_Core_STM32

STM32 core support for Arduino
https://github.com/stm32duino/Arduino_Core_STM32/wiki
Other
2.86k stars 980 forks source link

[Nucleo L432KC] `USER_BTN` is a misleading alias for pin `D9` #1726

Closed zfields closed 2 years ago

zfields commented 2 years ago

Describe the bug

There is only one button on the Nucleo L432KC, yet USER_BTN is defined in the variant file.

Based on my tinkering, I would say the USER_BTN is "pressed" by triggering the accelerometer. While I can see this as a handy feature, I think it is very misleading to call it USER_BTN, instead of ACCEL_PIN (or something of that nature).

To Reproduce

Complete source code which can be used to reproduce the issue. Please try to be as generic as possible (no extra code, extra hardware,...)

#define BUTTON USER_BTN // no user button available (triggered by accelerometer event)
// #define BUTTON 3 // behaves as expected with external momentary push button

volatile bool isr_fired = false;

void buttonISR () {
    isr_fired = true;
}

void setup () {
    ::pinMode(LED_BUILTIN, OUTPUT);
    ::pinMode(BUTTON, INPUT);
    ::attachInterrupt(digitalPinToInterrupt(BUTTON), buttonISR, FALLING);
}

void loop () {
    if (isr_fired) {
        isr_fired = false;
        ::digitalWrite(LED_BUILTIN, HIGH);
        ::delay(250);
    }
    ::digitalWrite(LED_BUILTIN, LOW);
}

Steps to reproduce the behavior:

  1. Plug
  2. Press USER_BTN (i.e. move or tap board)
  3. See error

Expected behavior A clear and concise description of what you expected to happen.

Screenshots

From the Product Overview Page: image

Desktop (please complete the following information):

Board (please complete the following information):

Additional context Add any other context about the problem here.

ABOSTM commented 2 years ago

Hi @zfields , I don't understand your point: 1) on Nucleo L432K, there is no accelerometer. So no reason to change this name.

2) USER_BTNis a generic name that is always defined (for all boards even if it is defined as PNUM_NOT_DEFINED) So this name will always exist and no rename possible.

Maybe you are talking about swan board which has accelerometer ? Whatever, you can redefine the name has you want: #define ACCEL_PIN USER_BTN or directly #define ACCEL_PIN PA8

fpistm commented 2 years ago

Hi, Before 2.0.0, USR_BTN was set to NC (Not connected): https://github.com/stm32duino/Arduino_Core_STM32/commit/d61905e32420e168305428a5d0471529d817d2f8#diff-7b96e366e38099ab783dc540dc5362de705b16cae00b840cc19fde34993568c6L64

Then after this PR #1091 it has been changed: https://github.com/stm32duino/Arduino_Core_STM32/pull/1091/commits/d61905e32420e168305428a5d0471529d817d2f8#diff-1bf8654ddb199d67f0d818c16569bf55e48cb8d88e422d4f799c14fa707832c3R64

So the only things we can do is to set it as PNUM_NOT_DEFINED.

USER_BTN is only a define and has no effect on the pin behavior :wink: simply ignore it and use your own definition.

zfields commented 2 years ago

Where exactly is PA8 exposed on the Nucleo L432KC (the board shown in the picture above - not a Swan)? There is no external button that I can see, other than the reset button.

on Nucleo L432K, there is no accelerometer.

I am blown away by this. Using the sketch I provided above with the Nucleo L432KC, the LED, LED_BUILTIN, only turns on when the device is tapped, moved or changed in orientation.

I guess I am proposing that USER_BTN should be evaluated as PNUM_NOT_DEFINED.

If I'm correct about an onboard accelerometer with an interrupt tied to PA8 and you wish to expose it, then it should have a name that makes sense, like ACCEL_INT_PIN. I only discovered the behavior while playing with the board, but I have no use of it. It's very easy to test with the firmware I provided. If I am mistaken, then simply disregard the suggestion.

fpistm commented 2 years ago

Ther is no accelerometer or sensor on this board. I guess you touch the PA8 pin then it detects a change.

ABOSTM commented 2 years ago

For sure, no accelerator on the board. Then you are right, there is no user button on that board, and theoretically USER_BTN should be evaluated as PNUM_NOT_DEFINED. Nevertheless I don't want to change USER_BTN definition, because it should not be an issue for users, and on the contrary if I change the definition it may cause trouble to people using this definition.

zfields commented 2 years ago

I found this on the internet and it appears to be correct (at least with regard to PA8).

image

So, USER_BTN is tied to the pin marked D9 on the Nucleo L432KC.

Nevertheless I don't want to change USER_BTN definition, because it should not be an issue for users...

I disagree. Literally, this exact situation led me to create this issue on your repository.

PA8 does not have the internal pull-up enabled by default. The pin is floating, so the interrupt fires every time the board is tapped, touched or tilted (basically acting like an accelerometer event).

While writing this I have discovered the source of my spurious interrupt events.

I originally used ::pinMode(BUTTON, INPUT_PULLUP);, but had changed the code while testing a different board to match the code above, ::pinMode(BUTTON, INPUT);. Even if my code was written as it was originally, I still would have been confused as to which pin is associated with PA8 (a.k.a. D9) and wondering where the USER button was supposed to be.

ABOSTM commented 2 years ago

The root cause of this issue, is not the wrong definition of USER_BTN, but rather that you tried to use a button that doesn't exist. And I still think that changing it would raise issue due to compatibility break.

zfields commented 2 years ago

Well, pin D9 does exist.

This issue will continue to present this issue for anyone who is porting an application to Nucleo L432KC from a different board (e.g. Swan R5) where USER_BTN is configured as INPUT with an external hardware pull-up.

// Relies on external hardware pull-up
::pinMode(USER_BTN, INPUT);