4 LEDs to 4 buttons

Hello,

I made a code from the button sample code from the nrf connect sdk where if each button lights up there corresponding led ex. button 0 turns on led 0.

/*
 * Copyright (c) 2016 Open-RnD Sp. z o.o.
 * Copyright (c) 2020 Nordic Semiconductor ASA
 *
 * SPDX-License-Identifier: Apache-2.0
 */

#include <zephyr.h>
#include <device.h>
#include <drivers/gpio.h>
#include <sys/util.h>
#include <sys/printk.h>
#include <inttypes.h>

#define SLEEP_TIME_MS	1

/*
 * Get button configuration from the devicetree sw0 alias. This is mandatory.
 */
#define SW0_NODE DT_ALIAS(sw0)
#define SW1_NODE DT_ALIAS(sw1)
#define SW2_NODE DT_ALIAS(sw2)
#define SW3_NODE DT_ALIAS(sw3)
#define LED0_NODE DT_ALIAS(led0)
#define LED1_NODE DT_ALIAS(led1)
#define LED2_NODE DT_ALIAS(led2)
#define LED3_NODE DT_ALIAS(led3)

#if !DT_NODE_HAS_STATUS(SW0_NODE, okay)
#error "Unsupported board: sw0 devicetree alias is not defined"
#endif

static const struct gpio_dt_spec button[] = {
	GPIO_DT_SPEC_GET_OR(SW0_NODE, gpios, {0}),
	GPIO_DT_SPEC_GET_OR(SW1_NODE, gpios, {0}),
	GPIO_DT_SPEC_GET_OR(SW2_NODE, gpios, {0}),
	GPIO_DT_SPEC_GET_OR(SW3_NODE, gpios, {0})
};

static struct gpio_callback button_cb_data;

/*
 * The led0 devicetree alias is optional. If present, we'll use it
 * to turn on the LED whenever the button is pressed.
 */
static struct gpio_dt_spec led[] = {
	GPIO_DT_SPEC_GET_OR(LED0_NODE, gpios,{0}),
	GPIO_DT_SPEC_GET_OR(LED1_NODE, gpios,{0}),
	GPIO_DT_SPEC_GET_OR(LED2_NODE, gpios,{0}),
	GPIO_DT_SPEC_GET_OR(LED3_NODE, gpios,{0})
};

void button_pressed(const struct device *dev, struct gpio_callback *cb,
		    uint32_t pins)
{
	printk("Button pressed at %" PRIu32 "\n", k_cycle_get_32());
}

void main(void)
{
	int ret;
	int led_num = ARRAY_SIZE(led);
	int button_num = ARRAY_SIZE(button);
	// printk("here 1\n");
	if (!device_is_ready(button[0].port)) {
		printk("Error: button device %s is not ready\n",
		       button[0].port->name);
		return;
	}

	for (int j=0; j < button_num; j++){
		ret = gpio_pin_configure_dt(&button[j], GPIO_INPUT);
		if (ret != 0) {
		printk("Error %d: failed to configure %s pin %d\n",
		       ret, button[j].port->name, button[j].pin);
		return;
		}
	}

	for (int k=0; k < button_num; k++){
		ret = gpio_pin_interrupt_configure_dt(&button[k], GPIO_INT_EDGE_TO_ACTIVE);
		if (ret != 0) {
		printk("Error %d: failed to configure interrupt on %s pin %d\n",
			ret, button[k].port->name, button[k].pin);
		return;
		}
	}
	
	for (int l=0; l < button_num; l++){
		gpio_init_callback(&button_cb_data, button_pressed, BIT(button[l].pin));
		gpio_add_callback(button[l].port, &button_cb_data);
		printk("Set up button at %s pin %d\n", button[l].port->name, button[l].pin);
	}

	for (int m=0; m < led_num; m++){

		if (led[m].port && !device_is_ready(led[m].port)) {
			printk("Error %d: LED device %s is not ready; ignoring it\n",
				ret, led[m].port->name);
			led[m].port = NULL;
		}
		if (led[m].port) {
			ret = gpio_pin_configure_dt(&led[m], GPIO_OUTPUT);
			if (ret != 0) {
				printk("Error %d: failed to configure LED device %s pin %d\n",
					ret, led[m].port->name, led[m].pin);
				led[m].port = NULL;
			} else {
				printk("Set up LED at %s pin %d\n", led[m].port->name, led[m].pin);
			}
		}
	}

	printk("Press the button\n");

	if (led[0].port) {
		while (1) {
			for (int n=0; n < led_num; n++){
				/* If we have an LED, match its state to the button's. */
				int val = gpio_pin_get_dt(&button[n]);

				if (val >= 0) {
					gpio_pin_set_dt(&led[n], val);
				}
				k_msleep(SLEEP_TIME_MS);
			}
			
		}
	}
}

I'd like to ask if there's a way to improve my code above? Right now, the code "works" but it seems that the button_pressed function print only works when I press button3

button_pressed code below:

void button_pressed(const struct device *dev, struct gpio_callback *cb,
		    uint32_t pins)
{
	printk("Button pressed at %" PRIu32 "\n", k_cycle_get_32());
}

What made it so far work is replacing the for gpio callback loop in the code:

for (int l=0; l < button_num; l++){
    gpio_init_callback(&button_cb_data, button_pressed, BIT(button[l].pin));
	gpio_add_callback(button[0].port, &button_cb_data);
	printk("Set up button at %s pin %d\n", button[0].port->name, button[l].pin);
}

with this:

gpio_init_callback(&button_cb_data_0, button_pressed, BIT(button[0].pin));
gpio_init_callback(&button_cb_data_1, button_pressed, BIT(button[1].pin));
gpio_init_callback(&button_cb_data_2, button_pressed, BIT(button[2].pin));
gpio_init_callback(&button_cb_data_3, button_pressed, BIT(button[3].pin));
gpio_add_callback(button[0].port, &button_cb_data_0);
gpio_add_callback(button[1].port, &button_cb_data_1);
gpio_add_callback(button[2].port, &button_cb_data_2);
gpio_add_callback(button[3].port, &button_cb_data_3);
for (int l=0; l < button_num; l++){
	printk("Set up button at %s pin %d\n", button[0].port->name, button[l].pin);
}

but this looks like not the proper way to do it. Hoping to hear how to go about this concern.

Board: nrf52840dk

Parents
  • Occasionally abstraction is unhelpful. The best way if you have the required resources free is:

    4x GPIOTE configured as inputs

    8x PPI events to transfer the ^^ HIGH/LOW events to...

    4x GPIOTE configured as outputs.

    See the datasheet for your chip and learn about those two features to wire it up.

  • New to the embedded space, from my code above how would you go about modifying it? a modified version of my code based on what you said would help me understand better

    4x GPIOTE configured as inputs

    Isn't this the one below based from my code above?

    for (int j=0; j < button_num; j++){
    		ret = gpio_pin_configure_dt(&button[j], GPIO_INPUT);
    		if (ret != 0) {
    		printk("Error %d: failed to configure %s pin %d\n",
    		       ret, button[j].port->name, button[j].pin);
    		return;
    		}
    	}

    8x PPI events to transfer the ^^ HIGH/LOW events to...

    It's my first time hearing PPI, what is it?

    4x GPIOTE configured as outputs.

    You mean the LEDs right? if yes, isn't it the one in my code?

    for (int m=0; m < led_num; m++){
    
    		if (led[m].port && !device_is_ready(led[m].port)) {
    			printk("Error %d: LED device %s is not ready; ignoring it\n",
    				ret, led[m].port->name);
    			led[m].port = NULL;
    		}
    		if (led[m].port) {
    			ret = gpio_pin_configure_dt(&led[m], GPIO_OUTPUT);
    			if (ret != 0) {
    				printk("Error %d: failed to configure LED device %s pin %d\n",
    					ret, led[m].port->name, led[m].pin);
    				led[m].port = NULL;
    			} else {
    				printk("Set up LED at %s pin %d\n", led[m].port->name, led[m].pin);
    			}
    		}
    	}

Reply
  • New to the embedded space, from my code above how would you go about modifying it? a modified version of my code based on what you said would help me understand better

    4x GPIOTE configured as inputs

    Isn't this the one below based from my code above?

    for (int j=0; j < button_num; j++){
    		ret = gpio_pin_configure_dt(&button[j], GPIO_INPUT);
    		if (ret != 0) {
    		printk("Error %d: failed to configure %s pin %d\n",
    		       ret, button[j].port->name, button[j].pin);
    		return;
    		}
    	}

    8x PPI events to transfer the ^^ HIGH/LOW events to...

    It's my first time hearing PPI, what is it?

    4x GPIOTE configured as outputs.

    You mean the LEDs right? if yes, isn't it the one in my code?

    for (int m=0; m < led_num; m++){
    
    		if (led[m].port && !device_is_ready(led[m].port)) {
    			printk("Error %d: LED device %s is not ready; ignoring it\n",
    				ret, led[m].port->name);
    			led[m].port = NULL;
    		}
    		if (led[m].port) {
    			ret = gpio_pin_configure_dt(&led[m], GPIO_OUTPUT);
    			if (ret != 0) {
    				printk("Error %d: failed to configure LED device %s pin %d\n",
    					ret, led[m].port->name, led[m].pin);
    				led[m].port = NULL;
    			} else {
    				printk("Set up LED at %s pin %d\n", led[m].port->name, led[m].pin);
    			}
    		}
    	}

Children
Related