Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable/disable connection check made with DTR #932

Merged
merged 7 commits into from
Aug 10, 2022
Merged

Enable/disable connection check made with DTR #932

merged 7 commits into from
Aug 10, 2022

Conversation

MrGreensWorkshop
Copy link
Contributor

@MrGreensWorkshop MrGreensWorkshop commented Jul 19, 2022

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.
@lurch
Copy link
Contributor

lurch commented Jul 19, 2022

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? 🤷

@MrGreensWorkshop
Copy link
Contributor Author

MrGreensWorkshop commented Jul 19, 2022

@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
Copy link
Contributor

lurch commented Jul 19, 2022

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?

@MrGreensWorkshop
Copy link
Contributor Author

MrGreensWorkshop commented Jul 19, 2022

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));

@kilograham kilograham added this to the 1.4.1 milestone Jul 20, 2022
@MrGreensWorkshop
Copy link
Contributor Author

MrGreensWorkshop commented Jul 20, 2022

@kilograham Thanks,
I added a fix,

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

if you want to revert let me know.

@kilograham
Copy link
Contributor

nice, thanks

@kilograham kilograham requested review from Wren6991 and liamfraser and removed request for Wren6991 July 21, 2022 01:01
@kilograham kilograham merged commit 80cde72 into raspberrypi:develop Aug 10, 2022
@MrGreensWorkshop MrGreensWorkshop deleted the feature_cdc_disable_dtr_option branch August 15, 2022 06:00
cniles pushed a commit to cniles/pico-sdk that referenced this pull request Aug 31, 2022
* Enable/disable connection check made with DTR
this gives users the option to disable DTR check.

Co-authored-by: Graham Sanderson <graham.sanderson@gmail.com>
@kilograham kilograham modified the milestones: 1.4.1, 1.5.0 Jan 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants