espressif / esp-rainmaker

ESP RainMaker Agent for firmware development
Apache License 2.0
432 stars 145 forks source link

button_handle_t takes only Active_Low Buttons (MEGH-4144) #218

Open exb53 opened 1 year ago

exb53 commented 1 year ago

I am not very experienced in programming the ESP32 with Rainmaker. In the gpio_button component seems to be a bug. It works only with active low buttons. So I did a little change in button.c. I replaced in lines 248/249

gpio_conf.pull_down_en = GPIO_PULLDOWN_DISABLE;
gpio_conf.pull_up_en = GPIO_PULLUP_ENABLE;

with: if (active_level == BUTTON_ACTIVE_LOW) { gpio_conf.pull_down_en = GPIO_PULLDOWN_DISABLE; gpio_conf.pull_up_en = GPIO_PULLUP_ENABLE; } else { gpio_conf.pull_down_en = GPIO_PULLDOWN_ENABLE; gpio_conf.pull_up_en = GPIO_PULLUP_DISABLE; } That works on both active low and active high button designs for me.

Please let me know, if there is any issue with that.

Thank you

shahpiyushv commented 1 year ago

@exb53 , This indeed seems to be a bug. However, this `gpio_button component will soon be deprecated as the same functionality is available in the updated button component. I see that it already has that fix here.

exb53 commented 1 year ago

Thank you, as I said I am not too experienced in programming . So should I delete the buttom‘s rainmaker include and use the iot‘s instead until a fixed rainmaker lib is available? Or should I stick with my fix?