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.

Sign up for GitHub

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

Jump to bottom

Move selection of UART interface and baudrate to picoprobe_config.h #7

Merged
merged 2 commits into from
Feb 3, 2021
Merged

Move selection of UART interface and baudrate to picoprobe_config.h #7

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c5d2e2e
Move selection of UART interface and baudrate to picoprobe_config.h
lurch Feb 2, 2021
4feec9d
Merge branch 'master' into uart_config
liamfraser Feb 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/cdc_uart.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
void cdc_uart_init(void) {
gpio_set_function(PICOPROBE_UART_TX, GPIO_FUNC_UART);
gpio_set_function(PICOPROBE_UART_RX, GPIO_FUNC_UART);
uart_init(uart1, 115200);
uart_init(PICOPROBE_UART_INTERFACE, PICOPROBE_UART_BAUDRATE);
}

#define MAX_UART_PKT 64
Expand All @@ -17,8 +17,8 @@ void cdc_task(void) {

// Consume uart fifo regardless even if not connected
uint rx_len = 0;
while(uart_is_readable(uart1) && (rx_len < MAX_UART_PKT)) {
rx_buf[rx_len++] = uart_getc(uart1);
while(uart_is_readable(PICOPROBE_UART_INTERFACE) && (rx_len < MAX_UART_PKT)) {
rx_buf[rx_len++] = uart_getc(PICOPROBE_UART_INTERFACE);
}

if (tud_cdc_connected()) {
Expand All @@ -33,12 +33,12 @@ void cdc_task(void) {
if (tud_cdc_available()) {
// Is there any data from the host for us to tx
uint tx_len = tud_cdc_read(tx_buf, sizeof(tx_buf));
uart_write_blocking(uart1, tx_buf, tx_len);
uart_write_blocking(PICOPROBE_UART_INTERFACE, tx_buf, tx_len);
}
}
}

void tud_cdc_line_coding_cb(uint8_t itf, cdc_line_coding_t const* line_coding) {
picoprobe_info("New baud rate %d\n", line_coding->bit_rate);
uart_init(uart1, line_coding->bit_rate);
}
uart_init(PICOPROBE_UART_INTERFACE, line_coding->bit_rate);
}
4 changes: 3 additions & 1 deletion src/picoprobe_config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,7 @@
// UART config
#define PICOPROBE_UART_TX 4
#define PICOPROBE_UART_RX 5
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This refers to the base code too, but what is the benefit in general of not using

PICO_DEFAULT_UART_BAUD_RATE (_TX_PIN, RX_PIN etc) as the default values for PICOPROBE values unless they are defined by the user

i;.e

#ifndef PICOPROBE_UART_TX
#define PICOPROBE_UART_TX PICO_DEFAULT_UART_TX_PIN
#endif

which i'd prefer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the answer to that depends on why @liamfraser opted to use UART pins 4 and 5 in the first place, instead of just sticking to the default 0 and 1 ? 🤷‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, if picoprobe_config.h switches to using

#ifndef PICOPROBE_UART_TX
#define PICOPROBE_UART_TX PICO_DEFAULT_UART_TX_PIN
#endif

where does the definition of a non-default PICOPROBE_UART_TX move to? CMakeLists.txt? Only allow it to be specified on the cmake command line? (I'm not being facetious, I'm genuinely curious)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it would from CMakeLists.txt (which could (should) choose to pick it up from the command line or environment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason it is done like this is so you can use the default uart as a debug uart and uart1 as the uart that binds to usb

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've never written any CMake before. But poking around in some of the other cmake files, would I want to do something like this?

if (DEFINED ENV{PICOPROBE_UART_TX} AND (NOT PICOPROBE_UART_TX))
    set(PICOPROBE_UART_TX $ENV{PICOPROBE_UART_TX})
    message("Using PICOPROBE_UART_TX from environment (${PICOPROBE_UART_TX})")
else ()
    set(PICOPROBE_UART_TX 4)
endif ()

🤷

#define PICOPROBE_UART_INTERFACE uart1
#define PICOPROBE_UART_BAUDRATE 115200

#endif
#endif