[Bug]: GPIO incorrect for Lilygo Lora32 2.1 1.6.1

[edwar64896]

Category

Other

Hardware

T-Lora v2 1.6

Firmware Version

2.5.20 (latest beta)

Description

have connected a switch across IO12 and GND as per doc, however this doesn't appear to have any effect as a user button. Continuity and circuit both checked. Am currently checking code for this board to see where the GPIO is configured...

PIN_BUTTON defined as '12' in Device Config (Android app)

Relevant log output

[edwar64896]

3ae85e2 appears to be the commit where BUTTON_PIN was nuked. I read somewhere that for the PIN_BUTTON configuration parameter to function, BUTTON_PIN must be defined...

src/input/ScanAndSelect.cpp

@ndoo ?

[edwar64896]

Further investigation: as this is my first foray into this repository, not entirely sure yet exactly where buttonGpio is getting handled for the Lilygo TLora32, but certainly, putting #define BUTTON_PIN 12 back into variant.h fixes the issue. I suspect though this isn't the end of the story.

[edwar64896]

ok here we are...

firmware/src/ButtonThread.cpp

Line 31 in b6562e1

#if defined(BUTTON_PIN) || defined(ARCH_PORTDUINO) || defined(USERPREFS_BUTTON_PIN)

Suspect that there are going to be more references like this meaning that really, BUTTON_PIN is going to have to be defined in variant.h for now unless someone wants to dig through the whole codebase to fix the dependency!

[ndoo]

Seems like the logic that makes the user definition not work needs to be fixed instead. Because restoring this causes electrical noise to trigger the GPIO since it's floating (as expected) since the board has no pull up/down resistors on the pin (as it was never intended to be used this way).

[todd-herbert]

The preferred way for tlora_v2_1_16 to add a user button would be by setting config.device.button_gpio. I'm seeing the same thing as you guys: it looks to me like setting this value won't have any impact if the variant doesn't originally define a BUTTON_PIN or USERPREFS_BUTTON_PIN.

Looking back through git blame, this one might be my fault. Sorry about that..

Fix button interrupt after light sleep (#3587)

• Don't assume device has a button..

This doesn't seem to have impacted NRF52 devices, because they tend to define PIN_BUTTON1 in variant.h, which is then assigned as BUTTON PIN here

firmware/src/platform/nrf52/architecture.h

Lines 104 to 106 in f0f2cd0

	#ifdef PIN_BUTTON1
	#define BUTTON_PIN PIN_BUTTON1
	#endif

I think the reason we're noticing this now is: a few months ago, the decision was made that things like BUTTON_PIN shouldn't be defined in variant.h unless they're stock hardware on a board. That attitude definitely makes sense to me, but as you guys had already spotted, it seems like the ButtonThread logic needs some updating to accommodate this.

As @edwar64896 also noticed, the doc about adding a button to LILYGO® TTGO Lora also needs updating.

Are either of you keen to work on changes to ButtonThread, or would you like me to take a look at it?

Edit: oh, I didn't realize this was already summed up nicely by #6222 (comment)

[edwar64896]

keen but about to jump into a 3 week project so will have bandwidth early April. If this isn't soon enough am happy to watch from the sidelines...