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

[BUG] STM32 compile error with TX_BUFFER_SIZE > 0 #16104

Closed
randellhodges opened this issue Dec 4, 2019 · 12 comments
Closed

[BUG] STM32 compile error with TX_BUFFER_SIZE > 0 #16104

randellhodges opened this issue Dec 4, 2019 · 12 comments

Comments

@randellhodges
Copy link
Contributor

Bug Description

Build error with STM32 and TX_BUFFER_SIZE > 0

Compiling .pio\build\BIGTREE_SKR_PRO\src\src\feature\controllerfan.cpp.o
In file included from c:\code\public\marlin\marlin\src\inc\marlinconfig.h:45:0,
                 from Marlin\src\feature\bedlevel\ubl\ubl.cpp:23:
Marlin\src\feature\bedlevel\ubl\ubl.cpp: In static member function 'static void unified_bed_leveling::display_map(int)':
c:\code\public\marlin\marlin\src\core\serial.h:81:44: error: 'class USBSerial' has no member named 'flushTX'; did you mean 'flush'?
   #define SERIAL_FLUSHTX()      SERIAL_OUT(flushTX)
                                            ^
c:\code\public\marlin\marlin\src\core\serial.h:62:50: note: in definition of macro 'SERIAL_OUT'
   #define SERIAL_OUT(WHAT, V...) (void)MYSERIAL0.WHAT(V)
                                                  ^~~~
Marlin\src\feature\bedlevel\ubl\ubl.cpp:214:9: note: in expansion of macro 'SERIAL_FLUSHTX'
         SERIAL_FLUSHTX();
         ^~~~~~~~~~~~~~
*** [.pio\build\BIGTREE_SKR_PRO\src\src\feature\bedlevel\ubl\ubl.cpp.o] Error 1

My Configurations

https://github.com/randellhodges/Marlin/tree/skr-1.1-pro/Marlin

Steps to Reproduce

  1. Build a SKR_PRO_1_1
  2. Set TX_BUFFER_SIZE to something larger than 0

Expected behavior: To compile

Actual behavior: Doesn't compile

Additional Information

From serial.h:

#ifdef __STM32F1__
  #define SERIAL_FLUSHTX()      SERIAL_OUT(flush)
#elif TX_BUFFER_SIZE > 0
  #define SERIAL_FLUSHTX()      SERIAL_OUT(flushTX)
#else
  #define SERIAL_FLUSHTX()
#endif

Looks like there is no flushTX for the STM32 HAL?

@xC0000005
Copy link
Contributor

ESP32 defines a class just to make a flushableSerial...that does not flush. it's literally there to make compile work, which sounds to me like a symptom of something else being broken, but we could tack the same thing in for STM32 if it makes this work.

@randellhodges
Copy link
Contributor Author

Not sure if it worth the effort to make the compile work. Then we'll all forget about it and never implement it.

There are some other checks on the STM32 platform for unsupported features, like emergency parser and what-not. Maybe just putting something there might be a better option for now.

@reloxx13
Copy link
Contributor

reloxx13 commented Dec 6, 2019

#15235

@boelle
Copy link
Contributor

boelle commented Dec 11, 2019

just a friendly poke here, but an idea on how we could fix this one?

@randellhodges
Copy link
Contributor Author

We could do what @xC0000005 suggested and create a dummy class. That'll prevent the error if someone enables that feature. It "hides" the underlying issue of not having a flushableSerial.

Then we could create a compile warning or something to people it isn't ready yet. The gets rid of the "oh, it won't compile" with a "oh, it isn't ready yet" scenario. (This is optional, but I'd hate to have people turn something on, think it works, when it doesn't really)

If we want to close out the bug, we could implement the workaround as suggested, then open a feature request to implement a flushableSerial for the STM32.

I am not certain what Marlin's philosophy is in this situation. Leaving it as compile bug so we know it needs to be implemented, or hide it until it is implemented. Based on @xC0000005 says about the ESP32 class, I'd say the later.

@boelle
Copy link
Contributor

boelle commented Dec 11, 2019

i'm not sure either

but yes hiding it seems logical, how would we do that? i'm not good at coding so if i should make a PR then i need to know what to add or replace and where

@boelle
Copy link
Contributor

boelle commented Dec 11, 2019

of course we need to make sure the core issue is not forgotten

@randellhodges
Copy link
Contributor Author

I can use the HAL_ESP32 as a guide and provide a PR for this.

@boelle
Copy link
Contributor

boelle commented Dec 11, 2019

sounds like a plan then :-)

@boelle
Copy link
Contributor

boelle commented Dec 24, 2019

was pr #16197 the one that fixes this one?

@randellhodges
Copy link
Contributor Author

All good

@github-actions
Copy link

github-actions bot commented Jul 3, 2020

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Jul 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants