CommunityGD32Cores / ArduinoCore-GD32

Arduino core for GD32 devices, community developed, based on original GigaDevice's core
Other
85 stars 33 forks source link

internal pullup and external interrupt issue #93

Closed flute2k3 closed 1 year ago

flute2k3 commented 1 year ago

The background is that I tried serval lib codes for EC11 encoder, but none of them works, after spent half a day I eventually find the internal pullup does not work at all for input pin, when I manually solder a pullup resistor it works all right. on the other hand, I can observe a 3.3V voltage at PA8, 9, 10, it is something around 0.3 ~ 0.6V.

please use below simplified codes to reproduce the issue, the EC11 encoder A, B to PA9, PA8, C grounded, switch to PA10, can short PA10 to simulate the press.

include

HardwareSerial Serial(PB7, PB6, 0);

void pinDidChange() { Serial.println("pinDidChange"); // pinMode(PA9, INPUT_PULLUP); //It works if I put these three codes here, works like a "reset", which is not elegant at all :-) // pinMode(PA8, INPUT_PULLUP); // pinMode(PA10, INPUT_PULLUP);

}

void prepare() { attachInterrupt(digitalPinToInterrupt(PA8), pinDidChange, CHANGE); attachInterrupt(digitalPinToInterrupt(PA9), pinDidChange, CHANGE); attachInterrupt(digitalPinToInterrupt(PA10), pinDidChange, CHANGE); }

void setup() {

Serial.begin(115200); Serial.println("EC11 Test"); // put your setup code here, to run once: pinMode(PA9, INPUT_PULLUP); pinMode(PA8, INPUT_PULLUP); pinMode(PA10, INPUT_PULLUP);

prepare(); }

void loop() {

delay(10); // put your main code here, to run repeatedly: }

maxgerhardt commented 1 year ago

With what platformio.ini is that exactly for reference?

We previously did have issues with pin modes being incorrect but I was convinced they're all properly fixed now in this file.

When you only do pinMode(PA9, INPUT_PULLUP);, as the only command in setup() and nothing in loop(), is the pullup still off? We could have a bug in attachInterrupt that resets the input mode back to INPUT after all. Exchanging the order of prepare() and pinMode(PA9, INPUT_PULLUP); may be a workaround for this, but if that's true, then it's a bug.

maxgerhardt commented 1 year ago

Yes it's exactly like this. We have a bug.

https://github.com/CommunityGD32Cores/ArduinoCore-GD32/blob/a178ad6ef661aff6b0b91245163ba4791f279a09/cores/arduino/gd32/gpio_interrupt.c#L48-L62

pinMode(x, INPUT_PULLUP); should work perfectly fine, the only problem is that we're resetting it to input mode (without pullup or pulldown) during the attachInterrupt process. Will be fixed.

flute2k3 commented 1 year ago

I use the board bluepill plus (https://github.com/WeActStudio/BluePill-Plus)

below is the ini file

[env:weact_bluepillplus_gd32f303cc] platform = gd32 board = weact_bluepillplus_gd32f303cc framework = arduino

flute2k3 commented 1 year ago

oid gpio_interrupt_enable(uint32_t portNum, uint32_t pinNum, vo

in the future in case if I suspect some thing wrong with a function, I can I locate the right place right file as you did, say for example the interrupt? maybe I can do some pre-study/pre-investigation to help with.

maxgerhardt commented 1 year ago

Could you remove the hardware pullups again and try this suggestion about the switched order?

#include <Arduino.h>

HardwareSerial Serial(PB7, PB6, 0);

void pinDidChange() {
Serial.println("pinDidChange");
}

void prepare() {
attachInterrupt(digitalPinToInterrupt(PA8), pinDidChange, CHANGE);
attachInterrupt(digitalPinToInterrupt(PA9), pinDidChange, CHANGE);
attachInterrupt(digitalPinToInterrupt(PA10), pinDidChange, CHANGE);
}

void setup() {

Serial.begin(115200);
Serial.println("EC11 Test");
// first setup interrupt (inits pin to "INPUT(_FLOATING)"
prepare();
// then activate their pullups
pinMode(PA9, INPUT_PULLUP);
pinMode(PA8, INPUT_PULLUP);
pinMode(PA10, INPUT_PULLUP);
}

void loop() {

delay(10);
// put your main code here, to run repeatedly:
}
flute2k3 commented 1 year ago

it works! but this looks not compatible with standard Arduino "style" :-)

maxgerhardt commented 1 year ago

Exactly, that's why we're going to fix this bug in our code here. I think what's either correct is to simply not do the gpio_init() to INPUT_FLOATING (but still turn on the GPIO clock in case that's the first time the pin was used and there's no pinMode() done previously on it), or try to read the old pullup/pulldown mode and set it as the same. The first thing should be easier becasue it's just code removal and the INPUT_FLOATING case is the default GPIO configuration in all microcontrollers I've read.

maxgerhardt commented 1 year ago

Can I ask you to pio pkg update -g -p gd32 again and test your original sketch, still without hardware pullups? I've tried to fix the bug in https://github.com/CommunityGD32Cores/ArduinoCore-GD32/commit/55934353dd9cd1d22d70c78e2222e512e77a89c6.

flute2k3 commented 1 year ago

work like a charm, thank you !