gh-raspberrypi/pico-sdk

Watch

Fork 0

Enable/disable connection check made with DTR #932

Merged

MrGreensWorkshop merged 7 commits from feature_cdc_disable_dtr_option into develop

2022-08-10 09:16:45 -05:00

MrGreensWorkshop commented

2022-07-19 02:16:14 -05:00

(Migrated from github.com)

This gives users the option to disable DTR check.
this fixes issues #906, #921

Problem:

tud_cdc_connected is using DTR bit which is used in stdio_usb.c If DTR is not set by host (PC terminal or other device), we can't send anything to host.

src/class/cdc/cdc_device.h

static inline bool tud_cdc_connected (void)
{
  return tud_cdc_n_connected(0);
}

src/class/cdc/cdc_device.h

bool tud_cdc_n_connected(uint8_t itf)
{
  // DTR (bit 0) active  is considered as connected
  return tud_ready() && tu_bit_test(_cdcd_itf[itf].line_state, 0);
}

how to fix

PICO_STDIO_USB_CONNECTION_WITHOUT_DTR option added to config file for who needs to enable.
It doesn't change current behavior because it's disabled by default. Just gives user an option to enable it.

Usage

add this to your CMakeLists.txt file and DTR check will be disabled.
add_definitions(-DPICO_STDIO_USB_CONNECTION_WITHOUT_DTR=1)

This gives users the option to disable DTR check. this fixes issues #906, #921 ### Problem: tud_cdc_connected is using DTR bit which is used in [stdio_usb.c](https://github.com/raspberrypi/pico-sdk/blob/master/src/rp2_common/pico_stdio_usb/stdio_usb.c) If DTR is not set by host (PC terminal or other device), we can't send anything to host. [src/class/cdc/cdc_device.h](https://github.com/hathach/tinyusb/blob/master/src/class/cdc/cdc_device.h) ``` static inline bool tud_cdc_connected (void) { return tud_cdc_n_connected(0); } ``` [src/class/cdc/cdc_device.h](https://github.com/hathach/tinyusb/blob/master/src/class/cdc/cdc_device.c) ``` bool tud_cdc_n_connected(uint8_t itf) { // DTR (bit 0) active is considered as connected return tud_ready() && tu_bit_test(_cdcd_itf[itf].line_state, 0); } ``` ### how to fix PICO_STDIO_USB_CONNECTION_WITHOUT_DTR option added to config file for who needs to enable. It doesn't change current behavior because it's disabled by default. Just gives user an option to enable it. ### Usage add this to your CMakeLists.txt file and DTR check will be disabled. add_definitions(-DPICO_STDIO_USB_CONNECTION_WITHOUT_DTR=1)

liamfraser (Migrated from github.com) reviewed

2022-07-19 02:16:14 -05:00

lurch commented

2022-07-19 06:15:03 -05:00

(Migrated from github.com)

I guess this might affect the behaviour of the PICO_STDIO_USB_CONNECT_WAIT_TIMEOUT_MS and PICO_STDIO_USB_POST_CONNECT_WAIT_DELAY_MS options? 🤷

I guess this might affect the behaviour of the `PICO_STDIO_USB_CONNECT_WAIT_TIMEOUT_MS` and `PICO_STDIO_USB_POST_CONNECT_WAIT_DELAY_MS` options? :shrug:

MrGreensWorkshop commented

2022-07-19 06:58:58 -05:00

(Migrated from github.com)

@lurch
It depends on future plans for stdio_usb_connected so I respect that and change the code.

I think you mean when PICO_STDIO_USB_CONNECTION_CHECK_WITH_DTR and those flags combined ?

how about this? when both flags are enabled adding sleep

bool stdio_usb_connected(void) {
#if PICO_STDIO_USB_CONNECTION_CHECK_WITH_DTR
    // this actually checks DTR
    return tud_cdc_connected();
#else
    #if PICO_STDIO_USB_POST_CONNECT_WAIT_DELAY_MS != 0
         sleep_ms(PICO_STDIO_USB_POST_CONNECT_WAIT_DELAY_MS);
    #endif
    return true;
#endif
}

@lurch It depends on future plans for `stdio_usb_connected` so I respect that and change the code. I think you mean when PICO_STDIO_USB_CONNECTION_CHECK_WITH_DTR and those flags combined ? how about this? when both flags are enabled adding sleep ``` bool stdio_usb_connected(void) { #if PICO_STDIO_USB_CONNECTION_CHECK_WITH_DTR // this actually checks DTR return tud_cdc_connected(); #else #if PICO_STDIO_USB_POST_CONNECT_WAIT_DELAY_MS != 0 sleep_ms(PICO_STDIO_USB_POST_CONNECT_WAIT_DELAY_MS); #endif return true; #endif } ```

lurch commented

2022-07-19 07:20:31 -05:00

(Migrated from github.com)

I dunno, it's entirely up to @kilograham , but I was just wondering if it might be e.g. worth doing a compile-time #warning if PICO_STDIO_USB_CONNECT_WAIT_TIMEOUT_MS is non-zero, and PICO_STDIO_USB_CONNECTION_CHECK_WITH_DTR is zero? 🤷

And IMHO you probably don't want a sleep inside of stdio_usb_connected, given how often it gets called?

I dunno, it's entirely up to @kilograham , but I was just wondering if it might be e.g. worth doing a compile-time `#warning` if `PICO_STDIO_USB_CONNECT_WAIT_TIMEOUT_MS` is non-zero, and `PICO_STDIO_USB_CONNECTION_CHECK_WITH_DTR` is zero? :shrug: And IMHO you probably _don't_ want a sleep inside of `stdio_usb_connected`, given how often it gets called?

MrGreensWorkshop commented

2022-07-19 07:31:00 -05:00

(Migrated from github.com)

to be clear PICO_STDIO_USB_CONNECTION_CHECK_WITH_DTRis default enabled. so sleep will be called if PICO_STDIO_USB_CONNECTION_CHECK_WITH_DTR disabled and if PICO_STDIO_USB_POST_CONNECT_WAIT_DELAY_MS is set other than 0.

in here, when stdio_usb_connected called, it is always returns true so, if PICO_STDIO_USB_POST_CONNECT_WAIT_DELAY_MS != 0 I though its best wait it in stdio_usb_connected.

        do {
            if (stdio_usb_connected()) {
#if PICO_STDIO_USB_POST_CONNECT_WAIT_DELAY_MS != 0
                sleep_ms(PICO_STDIO_USB_POST_CONNECT_WAIT_DELAY_MS);
#endif
                break;
            }
            sleep_ms(10);
        } while (!time_reached(until));

to be clear `PICO_STDIO_USB_CONNECTION_CHECK_WITH_DTR`is default enabled. so sleep will be called if `PICO_STDIO_USB_CONNECTION_CHECK_WITH_DTR` disabled and if `PICO_STDIO_USB_POST_CONNECT_WAIT_DELAY_MS` is set other than 0. in here, when `stdio_usb_connected` called, it is always returns true so, if PICO_STDIO_USB_POST_CONNECT_WAIT_DELAY_MS != 0 I though its best wait it in stdio_usb_connected. ``` do { if (stdio_usb_connected()) { #if PICO_STDIO_USB_POST_CONNECT_WAIT_DELAY_MS != 0 sleep_ms(PICO_STDIO_USB_POST_CONNECT_WAIT_DELAY_MS); #endif break; } sleep_ms(10); } while (!time_reached(until)); ```

MrGreensWorkshop commented

2022-07-20 17:39:15 -05:00

(Migrated from github.com)

@kilograham Thanks,
I added a fix,

There was a typo on comments. -> fixed
PICO_STDIO_USB_CONNECTION_WITHOUT_DTR was enabled by default -> disabled

if you want to revert let me know.

@kilograham Thanks, I added a fix, 1. There was a typo on comments. -> fixed 1. PICO_STDIO_USB_CONNECTION_WITHOUT_DTR was enabled by default -> disabled if you want to revert let me know.

kilograham commented

2022-07-20 20:01:37 -05:00

(Migrated from github.com)