change of mind wrt buttons

components/audio_controls/audio_controls.c

@@ -40,44 +40,44 @@ typedef struct {
 static audio_control_t default_control, current_control;
 
 /****************************************************************************************
- * Volume DOWN command
+ * DOWN button
  */
-static void volume_down(button_event_e event, button_press_e press) {
-	ESP_LOGI(TAG, "volume down %u", event);
-	//if (event == BUTTON_PRESSED) (*current_control.volume_down)();
-}
-
-static void volume_down_special(button_event_e event, button_press_e press) {
-	if (press == BUTTON_LONG) ESP_LOGI(TAG, "volume down long %u", event);
-	else ESP_LOGI(TAG, "volume down shifted %u", event);
+static void down(button_event_e event, button_press_e press, bool long_press) {
+	if (press == BUTTON_NORMAL) {
+		if (!long_press) ESP_LOGI(TAG, "volume DOWN %u", event);
+		else ESP_LOGI(TAG, "volume DOWN long %u", event);
+	} else {
+		if (!long_press) ESP_LOGI(TAG, "volume DOWN shifted %u", event);
+		else ESP_LOGI(TAG, "volume DOWN long shifted %u", event);
+	}
 	//if (event == BUTTON_PRESSED) (*current_control.volume_down)();
 }
 
 /****************************************************************************************
- * Volume UP commands
+ * UP button
  */
-static void volume_up(button_event_e event, button_press_e press) {
-	ESP_LOGI(TAG, "volume up %u", event);
+static void up(button_event_e event, button_press_e press, bool long_press) {
+	if (press == BUTTON_NORMAL) {
+		if (!long_press) ESP_LOGI(TAG, "volume UP %u", event);
+		else ESP_LOGI(TAG, "volume UP long %u", event);
+	} else {
+		if (!long_press) ESP_LOGI(TAG, "volume UP shifted %u", event);
+		else ESP_LOGI(TAG, "volume UP long shifted %u", event);
+	}
 	//if (event == BUTTON_PRESSED) (*current_control.volume_up)();
 }
 
-static void volume_up_long(button_event_e event, button_press_e press) {
-	if (press == BUTTON_LONG) ESP_LOGI(TAG, "volume up long %u", event);
-	else ESP_LOGI(TAG, "volume up shifted %u", event);
-	//if (event == BUTTON_PRESSED) (*current_control.volume_down)();
-}
-
 /****************************************************************************************
  * 
  */
 void audio_controls_init(void) {
 	/*
-	button_create(18, BUTTON_LOW, true, volume_up, NULL);
-	button_create(19, BUTTON_LOW, true, volume_down, "long", volume_down_long, 3000, NULL);
-	button_create(21, BUTTON_LOW, true, volume_up, NULL);
+	button_create(18, BUTTON_LOW, true, up, 0, -1);
+	button_create(19, BUTTON_LOW, true, down, 0, -1);
+	button_create(21, BUTTON_LOW, true, play, 0, -1);
 	*/
-	button_create(4, BUTTON_LOW, true, volume_up, NULL);
-	button_create(5, BUTTON_LOW, true, volume_down, "long", volume_down_special, 3000, "shift", volume_down_special, 4, NULL);
+	button_create(4, BUTTON_LOW, true, up, 2000, -1);
+	button_create(5, BUTTON_LOW, true, down, 3000, 4);
 }
 
 /****************************************************************************************

components/audio_controls/buttons.c

@@ -41,7 +41,7 @@ static int n_buttons = 0;
 
 static struct button_s {
 	int gpio, index;
-	button_handler handler, long_handler, shift_handler;
+	button_handler handler;
 	struct button_s *shifter;
 	int	long_press;
 	bool long_timer, shifted, shifting;
@@ -63,7 +63,7 @@ static void IRAM_ATTR gpio_isr_handler(void* arg)
 
 	if (xTimerGetPeriod(button->timer) > DEBOUNCE / portTICK_RATE_MS) xTimerChangePeriodFromISR(button->timer, DEBOUNCE / portTICK_RATE_MS, &woken); // does that restart the timer? 
 	else xTimerResetFromISR(button->timer, &woken);
-	// ESP_EARLY_LOGD(TAG, "INT gpio %u level %u", button->gpio, button->level);
+	// ESP_EARLY_LOGI(TAG, "INT gpio %u level %u", button->gpio, button->level);
 }
 
 /****************************************************************************************
@@ -71,7 +71,7 @@ static void IRAM_ATTR gpio_isr_handler(void* arg)
  */
 static void buttons_timer( TimerHandle_t xTimer ) {
 	struct button_s *button = (struct button_s*) pvTimerGetTimerID (xTimer);
-	
+
 	button->level = gpio_get_level(button->gpio);
 
 	if (button->long_press && !button->long_timer && button->level == button->type) {
@@ -97,7 +97,7 @@ static void buttons_task(void* arg) {
     while (1) {
 		struct button_s button;
 		button_event_e event;
-		bool shifted = false;
+		button_press_e press = BUTTON_NORMAL;
 
         if (!xQueueReceive(button_evt_queue, &button, portMAX_DELAY)) continue;
 
@@ -108,7 +108,7 @@ static void buttons_task(void* arg) {
 		// find if shifting is activated
 		if (button.shifter && button.shifter->type == button.shifter->level) {
 			button.shifter->shifting = true;
-			shifted = true;
+			press = BUTTON_SHIFTED;
 		} 
 	
 		/* 
@@ -119,32 +119,19 @@ static void buttons_task(void* arg) {
 		if (button.long_timer) {
 			if (event == BUTTON_RELEASED) {
 				// early release of a long-press button, send press/release
-				if (shifted) {
-					(*button.shift_handler)(BUTTON_PRESSED, BUTTON_SHIFTED);
-					(*button.shift_handler)(BUTTON_RELEASED, BUTTON_SHIFTED);
-				} else if (!button.shifting) {
-					(*button.handler)(BUTTON_PRESSED, BUTTON_NORMAL);
-					(*button.handler)(BUTTON_RELEASED, BUTTON_NORMAL);
-				}	
+				if (!button.shifting) {
+					(*button.handler)(BUTTON_PRESSED, press, false);		
+					(*button.handler)(BUTTON_RELEASED, press, false);		
+				}
 				// button is a copy, so need to go to real context
 				buttons[button.index].shifting = false;
 			} else if (!button.shifting) {
-				/* 
-				normal long press and not shifting so don't discard but there
-				is no long-press when shifted
-				*/
-				if (shifted) (*button.shift_handler)(BUTTON_PRESSED, BUTTON_SHIFTED);
-				else (*button.long_handler)(BUTTON_PRESSED, BUTTON_LONG);
+				// normal long press and not shifting so don't discard
+				(*button.handler)(BUTTON_PRESSED, press, true);
 			}  
-		} else if (shifted) {
-			// we are shifted
-			(*button.shift_handler)(event, BUTTON_SHIFTED);
 		} else {
 			// normal press/release of a button or release of a long-press button
-			if (!button.shifting) {
-				if (button.long_press) (*button.long_handler)(event, BUTTON_LONG);
-				else (*button.handler)(event, BUTTON_NORMAL);
-			}
+			if (!button.shifting) (*button.handler)(event, press, button.long_press);
 			// button is a copy, so need to go to real context
 			buttons[button.index].shifting = false;
 		}
@@ -161,13 +148,11 @@ void dummy_handler(button_event_e event, button_press_e press) {
 /****************************************************************************************
  * Create buttons 
  */
-void button_create(int gpio, int type, bool pull, button_handler handler, ...) { 
-	char *param;
-	va_list args;
+void button_create(int gpio, int type, bool pull, button_handler handler, int long_press, int shifter_gpio) { 
 	
 	if (n_buttons >= MAX_BUTTONS) return;
 
-	ESP_LOGI(TAG, "creating button using GPIO %u, type %u, pull-up/down %u", gpio, type, pull);
+	ESP_LOGI(TAG, "creating button using GPIO %u, type %u, pull-up/down %u, long press %u shifter %u", gpio, type, pull, long_press, shifter_gpio);
 
 	if (!n_buttons) {
 		button_evt_queue = xQueueCreate(10, sizeof(struct button_s));
@@ -181,40 +166,22 @@ void button_create(int gpio, int type, bool pull, button_handler handler, ...) {
 	// set mandatory parameters
  	buttons[n_buttons].gpio = gpio;
 	buttons[n_buttons].handler = handler;
+	buttons[n_buttons].long_press = long_press;
 	buttons[n_buttons].level = -1;
 	buttons[n_buttons].type = type;
 	buttons[n_buttons].timer = xTimerCreate("buttonTimer", DEBOUNCE / portTICK_RATE_MS, pdFALSE, (void *) &buttons[n_buttons], buttons_timer);
 	// little trick to find ourselves from queued copy
 	buttons[n_buttons].index = n_buttons;
 
-	va_start(args, handler);
-	
-	// set optional parameters
-	while ((param = va_arg(args, char*)) != NULL) {
-		if (!strcasecmp(param, "long")) {
-			buttons[n_buttons].long_handler = (button_handler) va_arg(args, void*);
-			buttons[n_buttons].long_press = va_arg(args, int);
-			ESP_LOGI(TAG, "adding long press %u (ms)", buttons[n_buttons].long_press);
-		} else if (!strcasecmp(param, "shift")) {
-			buttons[n_buttons].shift_handler = (button_handler) va_arg(args, void*);
-			int shifter_gpio = va_arg(args, int);
-			for (int i = 0; i < n_buttons; i++) {
-				if (buttons[i].gpio == shifter_gpio) {
-					buttons[n_buttons].shifter = buttons + i;
-					// a shifter must have a long-press handler
-					if (!buttons[i].long_press) {
-						buttons[i].long_press = 60*60*1000;
-						buttons[i].long_handler = dummy_handler;
-					} 
-					break;
-				}
-			}	
-			ESP_LOGI(TAG, "adding shifter %u", buttons[n_buttons].shifter->gpio);
+	for (int i = 0; i < n_buttons; i++) {
+		if (buttons[i].gpio == shifter_gpio) {
+			buttons[n_buttons].shifter = buttons + i;
+			// a shifter must have a long-press handler
+			if (!buttons[i].long_press) buttons[i].long_press = -1;
+			break;
 		}
-	}
+	}	
 
-	va_end(args);
-	
 	gpio_set_direction(gpio, GPIO_MODE_INPUT);
 
 	// we need any edge detection

components/audio_controls/buttons.h

@@ -23,20 +23,13 @@
 #define BUTTON_HIGH		1
 
 typedef enum { BUTTON_PRESSED, BUTTON_RELEASED } button_event_e; 
-typedef enum { BUTTON_NORMAL, BUTTON_LONG, BUTTON_SHIFTED } button_press_e; 
-typedef void (*button_handler)(button_event_e event, button_press_e mode);
+typedef enum { BUTTON_NORMAL, BUTTON_SHIFTED } button_press_e; 
+typedef void (*button_handler)(button_event_e event, button_press_e mode, bool long_press);
 
 /* 
-a button might have variable functions
-	- "long", <long_handler>, <duration_in_ms>
-	- "shift", <shift_handler>, <shifter_gpio>
-
-button_create(2, BUTTON_LOW, true, handler, NULL);
-button_create(5, BUTTON_HIGH, true, handler, "long", long_handler, 2000, NULL);
-button_create(6, BUTTON_LOW, true, handler, "shift", shift_handler, 5, NULL);
-button_create(6, BUTTON_HIGH, true, handler, "long", long_handler, 2000, "shift", shift_handler, 5);
-
+set long_press to 0 for no long-press
+set shifter_gpio to -1 for no shift
 NOTE: shifter buttons *must* be created before shiftee
 */
 
-void button_create(int gpio, int type, bool pull, button_handler handler, ...);
+void button_create(int gpio, int type, bool pull, button_handler handler, int long_press, int shifter_gpio);