usb: device: class: gs_usb: unify application event callbacks

Unify the application event callbacks to reduce code complexity and
simplify future event additions.

Signed-off-by: Henrik Brix Andersen <[email protected]>

app/src/cannectivity.h

@@ -35,37 +35,16 @@
 	DT_FOREACH_CHILD_STATUS_OKAY_SEP(CANNECTIVITY_DT_NODE_ID, fn, sep)
 
 /**
- * @brief CANnectivity CAN channel LED identify callback
+ * @brief CANnectivity CAN channel LED event callback
  *
  * @param dev Pointer to the device structure for the driver instance.
  * @param ch CAN channel number.
- * @param identify True if the channel identify is active, false otherwise.
+ * @param event Channel event.
  * @param user_data User data provided when registering the callback.
  * @return 0 on success, negative error number otherwise.
  */
-int cannectivity_led_identify(const struct device *dev, uint16_t ch, bool identify,
-			      void *user_data);
-
-/**
- * @brief CANnectivity CAN channel LED state callback
- *
- * @param dev Pointer to the device structure for the driver instance.
- * @param ch CAN channel number.
- * @param started True if the channel is started, false otherwise.
- * @param user_data User data provided when registering the callback.
- * @return 0 on success, negative error number otherwise.
- */
-int cannectivity_led_state(const struct device *dev, uint16_t ch, bool started, void *user_data);
-
-/**
- * @brief CANnectivity CAN channel LED activity callback
- *
- * @param dev Pointer to the device structure for the driver instance.
- * @param ch CAN channel number.
- * @param user_data User data provided when registering the callback.
- * @return 0 on success, negative error number otherwise.
- */
-int cannectivity_led_activity(const struct device *dev, uint16_t ch, void *user_data);
+int cannectivity_led_event(const struct device *dev, uint16_t ch, enum gs_usb_event event,
+			   void *user_data);
 
 /**
  * @brief CANnectivity LED initialization function

app/src/led.c

@@ -335,82 +335,60 @@ static void led_tick(struct k_timer *timer)
 	}
 }
 
-int cannectivity_led_identify(const struct device *dev, uint16_t ch, bool identify, void *user_data)
-{
-	led_event_t event;
-	int err;
-
-	ARG_UNUSED(dev);
-	ARG_UNUSED(user_data);
-
-	LOG_DBG("identify channel %u %s", ch, identify ? "on" : "off");
-
-	if (identify) {
-		event = LED_EVENT_CHANNEL_IDENTIFY_ON;
-	} else {
-		event = LED_EVENT_CHANNEL_IDENTIFY_OFF;
-	}
-
-	err = led_event_enqueue(ch, event);
-	if (err != 0) {
-		LOG_ERR("failed to enqueue identify %s event for channel %u (err %d)",
-			event == LED_EVENT_CHANNEL_IDENTIFY_ON ? "on" : "off", ch, err);
-	}
-
-	return 0;
-}
-
-int cannectivity_led_state(const struct device *dev, uint16_t ch, bool started, void *user_data)
-{
-	led_event_t event;
-	int err;
-
-	ARG_UNUSED(dev);
-	ARG_UNUSED(user_data);
-
-	LOG_DBG("channel %u %s", ch, started ? "started" : "stopped");
-
-	if (started) {
-		event = LED_EVENT_CHANNEL_STARTED;
-	} else {
-		event = LED_EVENT_CHANNEL_STOPPED;
-	}
-
-	err = led_event_enqueue(ch, event);
-	if (err != 0) {
-		LOG_ERR("failed to enqueue channel %s event for channel %u (err %d)",
-			event == LED_EVENT_CHANNEL_STOPPED ? "stopped" : "started", ch, err);
-	}
-
-	return 0;
-}
-
-int cannectivity_led_activity(const struct device *dev, uint16_t ch, void *user_data)
+int cannectivity_led_event(const struct device *dev, uint16_t ch, enum gs_usb_event event,
+			   void *user_data)
 {
+	led_event_t led_event;
 	struct led_ctx *lctx;
 	int err;
 
 	ARG_UNUSED(dev);
 	ARG_UNUSED(user_data);
 
 	if (ch >= ARRAY_SIZE(led_channel_ctx)) {
-		LOG_ERR("activity event for non-existing channel %u", ch);
+		LOG_ERR("event for non-existing channel %u", ch);
 		return -EINVAL;
 	}
 
 	lctx = &led_channel_ctx[ch];
 
-	/* low-pass filter activity events */
-	if (sys_timepoint_expired(lctx->activity)) {
+	switch (event) {
+	case GS_USB_EVENT_CHANNEL_STARTED:
+		LOG_DBG("channel %u started", ch);
+		led_event = LED_EVENT_CHANNEL_STARTED;
+		break;
+	case GS_USB_EVENT_CHANNEL_STOPPED:
+		LOG_DBG("channel %u stopped", ch);
+		led_event = LED_EVENT_CHANNEL_STOPPED;
+		break;
+	case GS_USB_EVENT_CHANNEL_ACTIVITY:
+		/* low-pass filter activity events */
+		if (!sys_timepoint_expired(lctx->activity)) {
+			goto skipped;
+		}
+
 		lctx->activity = sys_timepoint_calc(K_MSEC(LED_TICK_MS * LED_TICKS_ACTIVITY));
+		led_event = LED_EVENT_CHANNEL_ACTIVITY;
+		break;
+	case GS_USB_EVENT_CHANNEL_IDENTIFY_ON:
+		LOG_DBG("identify channel %u on", ch);
+		led_event = LED_EVENT_CHANNEL_IDENTIFY_ON;
+		break;
+	case GS_USB_EVENT_CHANNEL_IDENTIFY_OFF:
+		LOG_DBG("identify channel %u off", ch);
+		led_event = LED_EVENT_CHANNEL_IDENTIFY_OFF;
+		break;
+	default:
+		/* Unsupported event */
+		goto skipped;
+	}
 
-		err = led_event_enqueue(ch, LED_EVENT_CHANNEL_ACTIVITY);
-		if (err != 0) {
-			LOG_ERR("failed to enqueue channel activity event for channel %u (err %d)",
-				ch, err);
-		}
+	err = led_event_enqueue(ch, led_event);
+	if (err != 0) {
+		LOG_ERR("failed to enqueue LED event for channel %u (err %d)", ch, err);
 	}
 
+skipped:
 	return 0;
 }
 

app/src/main.c

@@ -23,9 +23,7 @@ static const struct gs_usb_ops gs_usb_ops = {
 	.timestamp = cannectivity_timestamp_get,
 #endif
 #ifdef CONFIG_CANNECTIVITY_LED_GPIO
-	.identify = cannectivity_led_identify,
-	.state = cannectivity_led_state,
-	.activity = cannectivity_led_activity,
+	.event = cannectivity_led_event,
 #endif
 #ifdef CONFIG_CANNECTIVITY_TERMINATION_GPIO
 	.set_termination = cannectivity_set_termination,

include/cannectivity/usb/class/gs_usb.h

@@ -507,6 +507,26 @@ struct gs_usb_host_frame_hdr {
 
 /** @} */
 
+/**
+ * @brief Channel events.
+ *
+ * @see gs_usb_event_callback_t
+ */
+enum gs_usb_event {
+	/** Channel started. */
+	GS_USB_EVENT_CHANNEL_STARTED,
+	/** Channel stopped. */
+	GS_USB_EVENT_CHANNEL_STOPPED,
+	/** Channel RX/TX activity. */
+	GS_USB_EVENT_CHANNEL_ACTIVITY,
+#if defined(CONFIG_USB_DEVICE_GS_USB_IDENTIFICATION) || defined(CONFIG_USBD_GS_USB_IDENTIFICATION)
+	/** Visual channel identification on. */
+	GS_USB_EVENT_CHANNEL_IDENTIFY_ON,
+	/** Visual channel identification off. */
+	GS_USB_EVENT_CHANNEL_IDENTIFY_OFF,
+#endif
+};
+
 /**
  * @brief Custom (random) MSOSv2 vendor code
  */
@@ -539,18 +559,6 @@ void gs_usb_register_vendorcode_callback(gs_usb_vendorcode_callback_t callback);
 typedef int (*gs_usb_timestamp_callback_t)(const struct device *dev, uint32_t *timestamp,
 					   void *user_data);
 
-/**
- * @brief Defines the callback signature for visually identifying a given CAN channel
- *
- * @param dev Pointer to the device structure for the driver instance.
- * @param ch CAN channel number.
- * @param identify True if the channel identify is active, false otherwise.
- * @param user_data User data provided when registering the callback.
- * @return 0 on success, negative error number otherwise.
- */
-typedef int (*gs_usb_identify_callback_t)(const struct device *dev, uint16_t ch, bool identify,
-					  void *user_data);
-
 /**
  * @brief Defines the callback signature for setting the bus termination of a given CAN channel
  *
@@ -576,26 +584,16 @@ typedef int (*gs_usb_get_termination_callback_t)(const struct device *dev, uint1
 						 bool *terminated, void *user_data);
 
 /**
- * @brief Defines the callback signature for reporting the state of a given CAN channel
- *
- * @param dev Pointer to the device structure for the driver instance.
- * @param ch CAN channel number.
- * @param started True if the channel is started, false otherwise.
- * @param user_data User data provided when registering the callback.
- * @return 0 on success, negative error number otherwise.
- */
-typedef int (*gs_usb_state_callback_t)(const struct device *dev, uint16_t ch, bool started,
-				       void *user_data);
-
-/**
- * @brief Defines the callback signature for reporting RX/TX activity of a given CAN channel
+ * @brief Defines the callback signature for reporting events for a given CAN channel
  *
  * @param dev Pointer to the device structure for the driver instance.
  * @param ch CAN channel number.
+ * @param event Event type.
  * @param user_data User data provided when registering the callback.
  * @return 0 on success, negative error number otherwise.
  */
-typedef int (*gs_usb_activity_callback_t)(const struct device *dev, uint16_t ch, void *user_data);
+typedef int (*gs_usb_event_callback_t)(const struct device *dev, uint16_t ch,
+				       enum gs_usb_event event, void *user_data);
 
 /**
  * @brief Callback operations structure.
@@ -611,14 +609,8 @@ struct gs_usb_ops {
 	/** Optional CAN channel get termination callback */
 	gs_usb_get_termination_callback_t get_termination;
 #endif
-#if defined(CONFIG_USB_DEVICE_GS_USB_IDENTIFICATION) || defined(CONFIG_USBD_GS_USB_IDENTIFICATION)
-	/** Optional CAN channel identify callback */
-	gs_usb_identify_callback_t identify;
-#endif
-	/** CAN channel state callback */
-	gs_usb_state_callback_t state;
-	/** CAN channel activity callback */
-	gs_usb_activity_callback_t activity;
+	/** CAN channel event callback */
+	gs_usb_event_callback_t event;
 };
 
 /**

subsys/usb/device/class/gs_usb.c

@@ -493,6 +493,7 @@ static int gs_usb_request_mode(const struct device *dev, uint16_t ch, int32_t tl
 	struct gs_usb_data *data = dev->data;
 	struct gs_usb_channel_data *channel;
 	can_mode_t mode = CAN_MODE_NORMAL;
+	enum gs_usb_event event;
 	uint32_t flags;
 	int err;
 
@@ -569,8 +570,10 @@ static int gs_usb_request_mode(const struct device *dev, uint16_t ch, int32_t tl
 		return -ENOTSUP;
 	}
 
-	if (data->ops.state != NULL) {
-		err = data->ops.state(dev, channel->ch, channel->started, data->user_data);
+	if (data->ops.event != NULL) {
+		event = channel->started ? GS_USB_EVENT_CHANNEL_STARTED :
+			GS_USB_EVENT_CHANNEL_STOPPED;
+		err = data->ops.event(dev, channel->ch, event, data->user_data);
 		if (err != 0) {
 			LOG_ERR("failed to report channel %u state change (err %d)", channel->ch,
 				err);
@@ -583,15 +586,11 @@ static int gs_usb_request_mode(const struct device *dev, uint16_t ch, int32_t tl
 static int gs_usb_request_identify(const struct device *dev, uint16_t ch, int32_t tlen,
 				   uint8_t *tdata)
 {
+#ifdef CONFIG_USB_DEVICE_GS_USB_IDENTIFICATION
 	struct gs_usb_identify_mode *im = (struct gs_usb_identify_mode *)tdata;
 	struct gs_usb_data *data = dev->data;
+	enum gs_usb_event event;
 	uint32_t mode;
-	bool identify;
-
-	if (data->ops.identify == NULL) {
-		LOG_ERR("identify not supported");
-		return -ENOTSUP;
-	}
 
 	if (ch >= data->nchannels) {
 		LOG_ERR("identify request for non-existing channel %u", ch);
@@ -607,17 +606,20 @@ static int gs_usb_request_identify(const struct device *dev, uint16_t ch, int32_
 
 	switch (mode) {
 	case GS_USB_CHANNEL_IDENTIFY_MODE_OFF:
-		identify = false;
+		event = GS_USB_EVENT_CHANNEL_IDENTIFY_OFF;
 		break;
 	case GS_USB_CHANNEL_IDENTIFY_MODE_ON:
-		identify = true;
+		event = GS_USB_EVENT_CHANNEL_IDENTIFY_ON;
 		break;
 	default:
 		LOG_ERR("unsupported identify mode %d for channel %u", mode, ch);
 		return -ENOTSUP;
 	}
 
-	return data->ops.identify(dev, ch, identify, data->user_data);
+	return data->ops.event(dev, ch, event, data->user_data);
+#else /* CONFIG_USB_DEVICE_GS_USB_IDENTIFICATION */
+	return -ENOTSUP;
+#endif /* !CONFIG_USB_DEVICE_GS_USB_IDENTIFICATION */
 }
 
 static int gs_usb_request_device_config(const struct device *dev, int32_t *tlen, uint8_t **tdata)
@@ -975,8 +977,9 @@ static void gs_usb_rx_thread(void *p1, void *p2, void *p3)
 			continue;
 		}
 
-		if (data->ops.activity != NULL) {
-			err = data->ops.activity(dev, ch, data->user_data);
+		if (data->ops.event != NULL) {
+			err = data->ops.event(dev, ch, GS_USB_EVENT_CHANNEL_ACTIVITY,
+					      data->user_data);
 			if (err != 0) {
 				LOG_ERR("activity callback for channel %u failed (err %d)", ch,
 					err);
@@ -1163,7 +1166,7 @@ static uint32_t gs_usb_features_from_ops(struct gs_usb_ops *ops)
 		features |= GS_USB_CAN_FEATURE_HW_TIMESTAMP;
 	}
 
-	if (ops->identify != NULL) {
+	if (UTIL_AND(IS_ENABLED(CONFIG_USB_DEVICE_GS_USB_IDENTIFICATION), ops->event != NULL)) {
 		features |= GS_USB_CAN_FEATURE_IDENTIFY;
 	}