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

Link errors when using -D PIO_FRAMEWORK_ARDUINO_NO_USB #1434

Closed
git2212 opened this issue May 9, 2023 · 13 comments
Closed

Link errors when using -D PIO_FRAMEWORK_ARDUINO_NO_USB #1434

git2212 opened this issue May 9, 2023 · 13 comments
Labels
waiting for feedback Requires response from original poster

Comments

@git2212
Copy link
Contributor

git2212 commented May 9, 2023

Compiling with this flag, e.g. when using pico-debug, using VS+Platformio will result in link errors in many libraries, e.g. TFT_eSPI, RPI_PICO_TimerInterrupt, as Serial is left undefined.

To avoid link errors in the many libraries where the authors assumed Serial is always present, perhaps Serial could be made a dummy (for lack of a better term) in case the PIO_FRAMEWORK_ARDUINO_NO_USB is added as a build define?

@earlephilhower
Copy link
Owner

pico-debug takes over 1 core and the USB interface, so Serial can't be used. IMHO making a dummy one that does nothing seems like it would just confuse folks who don't understand this.

If you really need libs which need Serial then do consider a standard Picoprobe or other external SWD debugger. They're really nice and don't use any onboard resources.

@maxgerhardt
Copy link
Contributor

maxgerhardt commented May 9, 2023

Hmm but in the NO_USB case could we not #define Serial Serial1?

@git2212 you can also try to just do that with macros,

build_flags =
  -DPIO_FRAMEWORK_ARDUINO_NO_USB 
  -DSerial=Serial1

in the platformio.ini.

@git2212
Copy link
Contributor Author

git2212 commented May 9, 2023

Some of the boards simply don't break out the pins to attach an external debugger, that's the situation I am in hence the need to use pico-debug.

Tried -DSerial=Serial1 but that leads down another path of compile errors with conflicting (re)declarations of Serial 1... e.g.

/home/arthur/.platformio/packages/framework-arduinopico/cores/rp2040/SerialUART.h:99:19: error: conflicting declaration SerialUART Serial1

While libraries aught not to make assumptions about availability of Serial, people do and just hard code Serial.print type statements in their libraries. This typically results in a compile time error for an undefined reference to Serial, but some of them use clever macros which avoid the compiler time errors and push the problem to the linker. When using the PIO_FRAMEWORK_ARDUINO_NO_USB currently, the Serial object has no implementation for the linker to link in, so that throws an error too... hence the recommendation for using a "dummy" Serial. perhaps with a #warning to boot so that everyone is aware of why they don't get an output as expected...

Confusion notwithstanding, if this is not handled at your level, every single library with this potential problem either needs to be refactored accordingly or is simply not an option because of this. I could be wrong but it seems it's easier to fix this at the core level than contact each library creator to fix their ill-behaved code... yeah I know, not your problem, but it is ours :)

From a dumb user standpoint, it is also reasonable to expect that the code should not break with debug compile if it works in normal compile mode

@git2212
Copy link
Contributor Author

git2212 commented May 9, 2023

The proposed solution is to add the code below to https://github.com/earlephilhower/arduino-pico/blob/master/cores/rp2040/SerialUSB.cpp just before the final #endif starting on line 216

#elif defined PIO_FRAMEWORK_ARDUINO_NO_USB
#warning "Using PIO_FRAMEWORK_ARDUINO_NO_USB. Serial will not output anything!"
#include <Arduino.h>
// #include <SerialUSB.h>
void SerialUSB::begin(unsigned long baud) {
}
void SerialUSB::end() {
}
int SerialUSB::peek() {
    return 0;
}
int SerialUSB::read() {
    return -1;
}
int SerialUSB::available() {
    return 0;
}
int SerialUSB::availableForWrite() {
    return 0;
}
void SerialUSB::flush() {

}
size_t SerialUSB::write(uint8_t c) {
    return 0;
}
size_t SerialUSB::write(const uint8_t *buf, size_t length) {
    return 0;
}
SerialUSB::operator bool() {
    return false;
}
SerialUSB Serial;

@earlephilhower earlephilhower reopened this May 9, 2023
@earlephilhower
Copy link
Owner

I'm still a little unconvinced, but this won't be the first compatibility hack in the core so I suppose I could hold my nose. 😆

If you could remove the #warning and (void) out unused params, feel free to submit a PR. (I hope most folks are building w/the default -Werror so the #warning and any unused method params will case build failures, or at least pollute build logs).

I'm not sure if this should be under the PIO_... macro, or the NO_USB option instead. The shorter version will cover PIO and Arduino builds, so that might make more sense because Arduino IDE can also upload to pico-debug.

git2212 added a commit to git2212/arduino-pico that referenced this issue May 9, 2023
@earlephilhower earlephilhower added the waiting for feedback Requires response from original poster label May 10, 2023
@maxgerhardt
Copy link
Contributor

Are we really sure there's not some Arduino convention that says Serial should point to an available serial object? Stubbing it out with a dummy implementation feels kind of wrong when it could be repointed to the first UART serial.

@git2212
Copy link
Contributor Author

git2212 commented May 10, 2023

The smaller boards this is an issue on don't break out the pins for more than one UART. Once you start debugging with pico-debug, you are out of (accessible) UARTs to redirect to. It's the tradeoff during the debug session, you have to give up the use of the one and only Serial.

@maxgerhardt
Copy link
Contributor

But that's weird, I thought pico-debug would leave the Hardware UART serials alone and only use the USB port for the Serial port. So accessing Serial1 (standard on pins GP0, GP1 but can differ per board definiiton) should work fine still.

@earlephilhower
Copy link
Owner

earlephilhower commented May 10, 2023

I think maybe he's only got one UART pinned out and using Serial1 in his code to talk to some device.

If we shift Serial1->Serial, Serial2->Serial1, then what happens is your code basically breaks when run in debug mode. And if we make Serial an alias for Serial1 then it also would basically break things because you'd end up mixing your HW protocol with those (USB-intended) Serial messages.

Is there an CMSIS/SWD type call to send chars down the debug pipe? Redirect USB to that SWD channel?

OTW, I think nulling it out might be the best thing to hope for.

@git2212
Copy link
Contributor Author

git2212 commented May 10, 2023

Correct, one UART is pinned out. Remapping Serial to Serial1 during the debug session would not be good in my case. The solution in the associated PR (blanking out Serial) is the solution for my use case. I can't seem to find the PR though, it just vanished?!?

@earlephilhower
Copy link
Owner

I can't seem to find the PR though, it just vanished?!?

I see you made a branch in your own fork, but you need to go to the GH web interface to actually turn that to a PR for this repo. If you go the your code website and then select your branch from the popup, I think GH will give you a 1-button box to make the PR.

@maxgerhardt
Copy link
Contributor

I.e., go to

master...git2212:arduino-pico:git2212-patch-1

and click "Create Pull Request"

@git2212
Copy link
Contributor Author

git2212 commented May 10, 2023

I.e., go to

master...git2212:arduino-pico:git2212-patch-1

and click "Create Pull Request"

ok, I seem to recall to have done that yesterday... oh well :) Thx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for feedback Requires response from original poster
Projects
None yet
Development

No branches or pull requests

3 participants