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

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

Merged
merged 2 commits into from
Feb 3, 2021

Conversation

lurch
Copy link
Contributor

@lurch lurch commented Feb 2, 2021

Fixes #1

@@ -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 ()

🤷

@liamfraser liamfraser merged commit c29510f into raspberrypi:master Feb 3, 2021
@lurch lurch deleted the uart_config branch February 3, 2021 09:24
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.

Configuring UART pins does not change hardware interface used
3 participants