HAL IRQ Shift with OpenBK7231T_App PR #1768 – Ongoing Testing and Results

Question

continuation of discussions and testing regarding the shift of interrupts to HAL. Related/origins: [line:95e128df5d] Tracking Platform Counter_f BK7231T ✅ BK7231N ✅ BK7231S/BK7231U ✅ BK7238 ✅ BK7252 BK7252N XR809 XR806 ✅ XR872/XF16 ✅ BL602/LF686 ✅ W800/W801 ✅ W600/W601 ✅ LN882H ✅...

p.kaczmarek2 · Accepted Answer

I have ESP32 set up and I will try to fix on ESP32 for now. Or rather... one crash fix will be applied

Added after 5 [minutes]:

Strange, I didn't push previous crash fix as well. Ok, both will compile soon. My PROTA is waiting:

PROTATool window showing commit history and GitHub pull request input

p.kaczmarek2 · Accepted Answer

BL0937 works because it either drives pin low or high, so no pull up/downs are needed.

But in case of testing, with either no button at all, or just a button (with no pull up/down resistors), the state on pin is undetermined, it's dangling in the air, so it receives noise and constantly triggers change.

That's why on some platforms you saw it increasing constantly.

It's no longer the case, now Counter is with pull up by default.

You can redo the tests on problematic platforms.

Which are missing now?

Added after 1 [minutes]:

@max4elektroda is pins config page still fetching each list of pin names many times, or did you optimize it? I wonder if we should have more Counter variations.

Next step is probably also OnChange interrupt with dynamic adjust in HAL implementation per platform....

I am creating multiplatform open source firmware (Tasmota replacement), right now supporting BK7231T, BK7231N, XR809, BL602, W800, W600, LN882H and soon supporting RTL and W701:
https://github.com/openshwprojects/OpenBK7231T
If you like my work, support me at: https://paypal.me/openshwprojects

Helpful post? Buy me a coffee.

p.kaczmarek2 · Answer

This should be easy to test, considering that most (if not all?) of dev boards have user-configurable button.

divadiow · Answer

ESP8266 1768_merge_a8731bd804dc seen at boot with Counter_f configured GPIO4 Code: TextLog in, to see the code then grounding IO4 = Code: TextLog in, to see the code

p.kaczmarek2 · Answer

Well, this indeed is called in previos bl0937 code:



#elif PLATFORM_ESPIDF || PLATFORM_ESP8266

   esp_cf = g_pins + GPIO_HLW_CF;
   esp_cf1 = g_pins + GPIO_HLW_CF1;
   gpio_install_isr_service(0);

Wait, but I do call it for ESP?

Screenshot of code editor showing interrupt handler logic for ESP8266/ESP-IDF

divadiow · Answer

XF16 ✅ Code: TextLog in, to see the code

p.kaczmarek2 · Answer

ah maybe it tries to remove twice, I pushed a guard: Probably HAL_DetachInterrupt gets called from PIN_SetPinRoleForPinIndex even when it was not attached earlier

divadiow · Answer

W600 ❌ no ACK of pin ground on PB8 when saving PB11 or PB12 Counter_f assignment: Code: TextLog in, to see the code RTS/PB10 saves but no gnd ack counter Added after 19 [minutes]: >>21640639 only one thing enabled for IRQ? IO_UART_RX Added after 4 [hours] 54 [minutes]: XR806 ✅ Code: TextLog in, to see the code Added after 1 [hours] 35 [minutes]: BK7231U ❌ Code: TextLog in, to see the code then it reboots. It will keep counting up between reboots each time grounding P9. Added after 12 [minutes]: ESP32 ❌ P14 Code: TextLog in, to see the code Added after 9 [minutes]:W800 Couldn't get it to acknowledge any assignments tried. How do I know which pins on any platform are expected to work? Added after 2 [minutes]:And maybe interrupts didn't work before HAL move for some platforms anyway

divadiow · Answer

these gets of packages every build is a pain Code: TextLog in, to see the code

p.kaczmarek2 · Answer

Any way to optimize it, @insmod ? I can't see any reasonable now, unless we try to use single machine (job) to build multiple platforms.

The good trick is to comment out not needed branches during testing in pr...

Ok, ESP32 works for me:

OpenESP32 web interface with system status and control buttons

insmod · Answer

>>21641096
I intend to use GCC that are already stored on github, like with XR806 https://github.com/openshwprojects/OpenBK7231...3e7d4befc5e94c9bf991fcb4c290d77/Makefile#L374
What i need to figure out is which GCC version is better for each SDK.

Regarding multiple platforms on single machine, i want to build various variants on a single machine, since variant changes are not in SDK but in OBK. Would also cut build time.
Current idea is to store manifest yamls in platform folders, and workflow would get them from there.

divadiow · Answer

same - 1768_merge_fba65afdaff9_4M Code: TextLog in, to see the code

p.kaczmarek2 · Answer

@divadiow do you get +1 for each button press? For me, it's not always +1. Probably it needs a capacitor to debounce the contacts.

Now, what about BK7231U ? Did moving the long code out of ISR helped there? Currently, I just increase a variable in ISR and then add it to channels in quick tick.

divadiow · Answer

no. it did seem to count up very quickly with what should have been 1 clean +1, I had no gpio button but was grounding P14.

p.kaczmarek2 · Answer

Regarding w600 and w800 - currently, I got this:

Log in, to see the code

So w800 is not included? Let me try to enable it.

Added after 18 [seconds]:

Also, it doesn't say why W600 does not work? Hmmm

Added after 3 [minutes]:

The used functions seem to be in w800 as well.
https://github.com/search?q=repo%3Aopenshwpro...penW800%20tls_gpio_isr_register&type=code

Log in, to see the code

Same in w600:
https://github.com/search?q=repo%3Aopenshwpro...penW600%20tls_gpio_isr_register&type=code
According to wm_gpio_demo.c:

Log in, to see the code

Hmm.... I do not configure it as input!

divadiow · Answer

1768_merge_fba65afdaff9 - seems OK now. no reboot after first grounding. like ESP32, counts up very quickly, sensitive. Code: TextLog in, to see the code

p.kaczmarek2 · Answer

Let's try this way: It's discussable whether we should rely on another HAL or just put pin setup into HAL_AttachInterrupt. Futhermore, it brings pull up/down options...

divadiow · Answer

upon save of Config_f pin on PB8 on W600 with 1768_merge_e7accd30f28a Code: TextLog in, to see the code Added after 8 [minutes]:W800 1768_merge_e7accd30f28a no counter acknowledgment on any. tried PA1, PA4, PB4, PB10

p.kaczmarek2 · Answer

Was BL0937 working on W600? I think it was?

Excerpt from W600 documentation showing GPIO controller with interrupt support.

They have interrupts:
https://github.com/winnermicro/micropython/bl...05fbca6/ports/w60x/machine/machine_pin.c#L252

Log in, to see the code

Well I guess I am missing tls_clr_gpio_irq_status , but it does not explain the crash on config and silence on w800

divadiow · Answer

I have other W600/W800 options I guess. Could transplant TW-02 into something real maybe.

p.kaczmarek2 · Answer

@insmod any ideas? I am starting to think that my lazy cast of integer to void * may be at fault, cause I am running out of ideas now.. but again, no, it crashes at config time for @divadiow now

divadiow · Answer

OK, with W600 and initial finding it that saving PB8 as Counter_f doesn't cause crash but constantly triggers count Code: TextLog in, to see the code Added after 10 [minutes]: there are two devices in list apparently with W600/BL0937 combination Code: JSONLog in, to see the code Added after 3 [minutes]: so CF and CF1 need interrupt? If yes, PB11, PB13, PB16, PB18 should have been OK for those devices in the past?

p.kaczmarek2 · Answer

Strange? But I call tls_clr_gpio_irq_status ? Or that's not this commit? Added after 40 :Both CF and CF1 are using interrutps in BL0937.

divadiow · Answer

this order builds Code: C / C++Log in, to see the code

p.kaczmarek2 · Answer

Oh well... I was too busy with real life work and didn't get what you mean and I assumed that is what I pushed. That's of course a total newbie error, but I did quick fix and didn't even check. Well, ok, so I pushed a fix again, and I have a W800 at hand (the one gifted by @max4elektroda ? ), so I will try to check it once it builds or in the morning.

Added after 1 [hours] 23 [minutes]:

W800, PB21, with a button to a ground. When button is not pressed, it increases steadily, if pressed, it stops.

Added after 3 [minutes]:

Added pull up resistor 10k to VDD - seems to work better, only change on press? Almost always, sometimes not.

Breadboard with microcontroller, pushbutton, and 10k pull-up resistor connected to VDD

Added after 4 [minutes]:

Ah, this is because I am calling HAL_PIN_Setup_Input for Counter_f .... so... so with no pullup/pulldown, there is noise on pin, that's why it changes so fast... it's to be expected, to be honest. Now the question is what should be the default? Input pullup? Pulldown? No resistor?

Added after 6 [minutes]:

I am not sure, but I changed default to input pullup so it works better without any resistor on gpio... otherwise you manually have to add pullup or pulldown...

I am creating multiplatform open source firmware (Tasmota replacement), right now supporting BK7231T, BK7231N, XR809, BL602, W800, W600, LN882H and soon supporting RTL and W701:
https://github.com/openshwprojects/OpenBK7231T
If you like my work, support me at: https://paypal.me/openshwprojects

Helpful post? Buy me a coffee.

divadiow · Answer

Ok sure. I didn't really know, I just gave GPT all the files and asked it about what might be wrong I am curious about the differences between platforms and default low/highs on various pins and rising and falling edges and stuff. I don't understand how the single BL0937 driver could possibly cater for all the variations, and yet to date it seems to work OK for most devices with BL0937 Added after 34 [minutes]:Maybe that's dumb, it doesn't matter what the defaults are because the BL0937 driver will just set pin to what it needs to be?

max4elektroda · Answer

I'll check but I'm quite sure pin names are only fetched once on loading and stored in a JS variable.

p.kaczmarek2 · Answer

It seems so... nice, your contributions are very useful. I remember when this list was repeated once per select box. Code: text Expand Select all Copy to clipboard var r = [[ ,1],[Rel,1],[Rel_n,1],[Btn,2],[Btn_n,2],[LED,1],[LED_n,1],[PWM,1],[WifiLED,0],[WifiLED_n,0],[Btn_Tgl_All,0],[Btn_Tgl_All_n,0],[dInput,1],[dInput_n,1],[TglChanOnTgl,1],[dInput_NoPullUp,1],[dInput_NoPullUp_n,1],[BL0937SEL,0],[BL0937CF,0],[BL0937CF1,0],[ADC,1],[SM2135DAT,0],[SM2135CLK,0],[BP5758D_DAT,0],[BP5758D_CLK,0],[BP1658CJ_DAT,0],[BP1658CJ_CLK,0],[PWM_n,1],[IRRecv,0],[IRSend,0],[Btn_NextColor,0],[Btn_NextColor_n,0],[Btn_NextDimmer,0],[Btn_NextDimmer_n,0],[AlwaysHigh,0],[AlwaysLow,0],[UCS1912_DIN,0],[SM16703P_DIN,0],[Btn_NextTemperature,0],[Btn_NextTemperature_n,0],[Btn_ScriptOnly,0],[Btn_ScriptOnly_n,0],[DHT11,2],[DHT12,2],[DHT21,2],[DHT22,2],[CHT83XX_SDA,2],[CHT83XX_SCK,0],[SHT3X_SDA,2],[SHT3X_SCK,0],[SoftSDA,1],[SoftSCL,1],[SM2235DAT,1],[SM2235CLK,1],[BridgeFWD,1],[BridgeREV,1],[Btn_SmartLED,1],[Btn_SmartLED_n,1],[DoorSnsrWSleep,1],[DoorSnsrWSleep_nPup,1],[BAT_ADC,1],[BAT_Relay,1],[TM1637_DIO,1],[TM1637_CLK,1],[BL0937SEL_n,1],[DoorSnsrWSleep_pd,1],[SGP_CLK,0],[SGP_DAT,2],[ADC_Button,1],[GN6932_CLK,1],[GN6932_DAT,1],[GN6932_STB,1],[TM1638_CLK,1],[TM1638_DAT,1],[TM1638_STB,1],[BAT_Relay_n,1],[KP18058_CLK,1],[KP18058_DAT,1],[DS1820_IO,1],[PWM_ScriptOnly,1]];var  sr = r.map((e,i)=>{return e[0]+'#'+i}).sort(Intl.Collator().compare).map(e=>e.split('#'));function hide_show() {n=this.name;er=getElement('r'+n);ee=getElement('e'+n);ch=r[this.value][1];er.disabled = (ch

max4elektroda · Answer

Regarding pullup/pulldown:
Iirc in some codes for interrupt handling the pin was "set to the opposite" of the event: raise would pulldown pin, fall would pullup pin.

But especially on a "both" situation we might don't want this? In fact, it will depend on the input.
Eg the DCF77 module will give a high pulse of a defined length for 0 or 1, so I used pulldown here, even when eg W800 also has "floating" - probably the cause of the strange behavior you noticed.

Added after 30 [minutes]:

I had another PR for this ;-)

My idea was to put all pin/role related config in one place, a simple text file, and generate a header file to be included instead of defining the enum here, the http names there...
It's also possible to add things like "channel usage", but that's optional.
Let me check the PR and its state not sure if i added the channel us in latest or not

HAL IRQ Shift with OpenBK7231T_App PR #1768 – Ongoing Testing and Results

Post #1

Post #2

Post #3

Post #4

Post #5

Post #6

Post #7

Post #8

Post #9

Post #10

Post #11

Post #12

Post #13

Post #14

Post #15

Post #16

Post #17

Post #18

Post #19

Post #20

Post #21

Post #22

Post #23

Post #24

Post #25

Post #26

Post #27

Post #28

Post #29

Post #30

Topic summary

Platform	Counter_f
BK7231T	✅
BK7231N	✅
BK7231S/BK7231U	✅
BK7238	✅
BK7252
BK7252N
XR809
XR806	✅
XR872/XF16	✅
BL602/LF686	✅
W800/W801	✅
W600/W601	✅
LN882H	✅
ESP8285/ESP8266	✅
ESP32	✅
C2
C3
C5
C6
C61
S2
S3
TR6260
RTL8711AM (Ameba1)
RTL8710B (AmebaZ)	✅
RTL8710C/RTL8720C (AmebaZ2)	✅
RTL8720D	✅
RTL872xCSM/RTL8720CS
RTL8721DA/RTL8711DAF	✅
RTL8720E/RTL8710ECF
ECR6600	✅