Addition of USB CDC ACM DTR bit change

[robhelvestine]

Introduction

Currently, the USB CDC ACM files (cdc_acm.h and cdc_acm.c) support a callback that is used for dte rate changes. I would like to follow this implementation to add another callback that notifies the user if the DTR bit changes. This allows users to know when a connection has been made / broken to a host computer that uses the USB CDC ACM device.

Problem description

Certain applications require knowing when a connection has been made or broken with a host device that uses the CDC ACM device. Using the DTR bit is the most accurate way to determine this, and this can't just be checked once on startup.

Proposed change

A callback will be added to the cdc_acm.h file to support changes to the DTR bit. Any user can set a callback that is called when this bit changes (0 -> 1 or 1 -> 0). This callback set will follow the dte_rate callback implementation by utilizing a Kconfig and appropriate #if 's

Detailed RFC

Following the flow of the dte_rate callback implementation is the clearest example of how this new callback would be implemented.

Proposed change (Detailed)

I've already written this patch and tested it on custom hardware (nrf53 MCU). Files changed: include/zephyr/drivers/uart/cdc_acm.h subsys/usb/device/class/Kconfig.cdc subsys/usb/device/class/cdc_acm.c changes to the function cdc_acm_class_handle_req shown below:

Dependencies

None. The callback can be NULL and therefore will never be called.

Concerns and Unresolved Questions

None. I have tested this, and it works as expected.

Alternatives

I believe this is the cleanest solution to monitor the DTR bit in the CDC ACM connection.

[robhelvestine]

@raykamp for visibility

[carlescufi]

@robhelvestine have you also checked the implementation in the new USB-next stack? https://github.com/zephyrproject-rtos/zephyr/blob/main/subsys/usb/device_next/class/usbd_cdc_acm.c

[robhelvestine]

Hey @carlescufi Thanks I hadn't noticed that. I've been working on Zephyr 3.2 recently, and this got implemented on 3.3. Anyway, I think a change to support a callback is still a useful addition to the codebase.

[nordicjm]

Certain applications require knowing when a connection has been made or broken with a host device that uses the CDC ACM device. Using the DTR bit is the most accurate way to determine this, and this can't just be checked once on startup.

Note that this is not true, the DTR state changing does not signify that there is an application connected

[robhelvestine]

@nordicjm Can you elaborate? From my research, it's the best way to determine connection between an embedded device and a PC using CDC ACM.

[nordicjm]

@tmon-nordic can provide more information. Essentially it is a feature that a serial program can set (if it supports it), it does not have to set it and it being set does not mean it was set by a client, it could be done by a driver or the OS itself. Likewise, when a program closes the port, it does not have to unset it, the program might leave the bit set.

Another good example would be brltty, which is a braile background application, which opens every tty device on the computer to see if it is a accessibility device and, likely, changes DTR state.

[robhelvestine]

@nordicjm Interesting. Thanks for the explanantion. Just to confirm, are you against adding this callback functionality for the DTR bit? I still see the value. User's don't need to implement the callback if they don't want to. But for embedded devices I've worked on (using tio or putty or teraterm as the serial connection), I have found this to be quite useful.

[nordicjm]

@nordicjm Interesting. Thanks for the explanantion. Just to confirm, are you against adding this callback functionality for the DTR bit? I still see the value. User's don't need to implement the callback if they don't want to. But for embedded devices I've worked on (using tio or putty or teraterm as the serial connection), I have found this to be quite useful.

Not against adding this so long as the PR does not make any mention of this being how to check for ports being opened on a PC, because that claim would not be true.

[carlescufi]

@robhelvestine why do you need the callback? The UART API already provides a way of waiting for DTR:

https://github.com/zephyrproject-rtos/zephyr/blob/2e3193e04d0bbfc5f074efae68f70a022c308934/samples/subsys/usb/cdc_acm/src/main.c#L178-L188

[carlescufi]

I understand that today we have an inconsistency, in this form:

DTE changes can either come from: https://github.com/zephyrproject-rtos/zephyr/blob/2e3193e04d0bbfc5f074efae68f70a022c308934/include/zephyr/drivers/uart/cdc_acm.h#L31

or https://github.com/zephyrproject-rtos/zephyr/blob/2e3193e04d0bbfc5f074efae68f70a022c308934/include/zephyr/drivers/uart.h#L1610 with https://github.com/zephyrproject-rtos/zephyr/blob/2e3193e04d0bbfc5f074efae68f70a022c308934/include/zephyr/drivers/uart.h#L34

However, we don't have that for DTR, where the only option is https://github.com/zephyrproject-rtos/zephyr/blob/2e3193e04d0bbfc5f074efae68f70a022c308934/include/zephyr/drivers/uart.h#L1610 with https://github.com/zephyrproject-rtos/zephyr/blob/2e3193e04d0bbfc5f074efae68f70a022c308934/include/zephyr/drivers/uart.h#L36

The USB-next stack will provide events for these cases, so we'd rather not extend the current stack for now.

[robhelvestine]

@robhelvestine why do you need the callback? The UART API already provides a way of waiting for DTR:

https://github.com/zephyrproject-rtos/zephyr/blob/2e3193e04d0bbfc5f074efae68f70a022c308934/samples/subsys/usb/cdc_acm/src/main.c#L178-L188

@carlescufi That code snippet only checks once. If you have multiple connections / disconnections after the embedded product is powered on, it will never be alerted to the user.

[robhelvestine]

I understand that today we have an inconsistency, in this form:

DTE changes can either come from:

https://github.com/zephyrproject-rtos/zephyr/blob/2e3193e04d0bbfc5f074efae68f70a022c308934/include/zephyr/drivers/uart/cdc_acm.h#L31

or

https://github.com/zephyrproject-rtos/zephyr/blob/2e3193e04d0bbfc5f074efae68f70a022c308934/include/zephyr/drivers/uart.h#L1610

with https://github.com/zephyrproject-rtos/zephyr/blob/2e3193e04d0bbfc5f074efae68f70a022c308934/include/zephyr/drivers/uart.h#L34

However, we don't have that for DTR, where the only option is

https://github.com/zephyrproject-rtos/zephyr/blob/2e3193e04d0bbfc5f074efae68f70a022c308934/include/zephyr/drivers/uart.h#L1610

with https://github.com/zephyrproject-rtos/zephyr/blob/2e3193e04d0bbfc5f074efae68f70a022c308934/include/zephyr/drivers/uart.h#L36

The USB-next stack will provide events for these cases, so we'd rather not extend the current stack for now.

@carlescufi You're correct. And understood.

[jfischer-no]

Also note that an application using this type of callback or any future CDC ACM specific notification is not generic. However, the virtual UART provided by the CDC ACM is generic and can be replaced by a real UART controller. This is why I was against this CDC ACM specific callback. Not sure if an RFC to add this functionality to the UART API is worth it, in any case it will not be worse than it is now. I will close it as not-planned.

[tmon-nordic]

The callback has the advantage over the polling in a loop in the way that it does not waste CPU. The DTR cannot really be used to determine whether application has connected because this is OS dependent. While you could do some heuristic for initial connection, the problem is when the application closes and leaves DTR intact.

[jfischer-no]

There are no plans to implement this suggestion. For the new USB device support, there are explicit messages for the CDC ACM UART line state and line coding events. https://github.com/zephyrproject-rtos/zephyr/blob/b3ffe0aede6db181e118eb95db84ad7537bed0b2/samples/subsys/usb/cdc_acm/src/main.c#L73-L84