Skip to content

[2.0.x] SERIAL_XON_XOFF not supported on USB-native AVR devices#8652

Merged
thinkyhead merged 1 commit intoMarlinFirmware:bugfix-2.0.xfrom
fiveangle:bf2_usbcon_xonxoff
Dec 4, 2017
Merged

[2.0.x] SERIAL_XON_XOFF not supported on USB-native AVR devices#8652
thinkyhead merged 1 commit intoMarlinFirmware:bugfix-2.0.xfrom
fiveangle:bf2_usbcon_xonxoff

Conversation

@fiveangle
Copy link
Copy Markdown

User could enable SERIAL_XON_XOFF on USB-native devices and it would not be enabled without warning, but M115 would report the capability as available.

User could enable SERIAL_XON_XOFF on USB-native devices and it would not be enabled without warning, but M115 would report the capability as available.
@fiveangle
Copy link
Copy Markdown
Author

@ejtagle - I assume Due wouldn't have any problem with how this is done ?

Also, you mentioned before the idea of improving flow control for USB-native via just halting acceptance of the data when the buffer was full, but what would be the downside of using the same flow control as on AVR ? It may not be quite as optimal, but wouldn't it be less implimentation effort on the host side ?

@ejtagle
Copy link
Copy Markdown
Contributor

ejtagle commented Dec 4, 2017

AVR with an USB native port (i don't have a board based on such device) could be made to just stall the RX endpoint, and that leads to "natural" flow control. It could be done.
I already have a Proof of Concept for the Due... My idea there was to reflect a Serial port and an MTP device (or an MSD) devicethat would allow to send content to the microSD

#error "TX_BUFFER_SIZE must be 0, a power of 2 greater than 1, and no greater than 256."
#endif
#elif ENABLED(SERIAL_XON_XOFF)
#error "SERIAL_XON_XOFF is not supported on USB-native AVR devices."
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This will never get triggered due to the #undef. Either the code should have the #undef with a warning, or just this #error.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I believe it was the other way around, the #undef SERIAL_XON_XOFF didn't get executed because compile didn't complete when SanityCheck threw the error. I left the redundant check in Conditional_adv.h since it didn't seem to hurt, and might prevent future issue if something ever caused the SanityCheck to break. But thinking about your comment, perhaps it is "smarter" to not try and be so smart, lest be met with unintended consequences (if compilation priority changes for some reason, for example). I'll double check anyway to make sure current behavior throws the error instead of silent undef.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Weird. The sanity check always comes after Conditionals_post.h.

@thinkyhead thinkyhead merged commit 37fff60 into MarlinFirmware:bugfix-2.0.x Dec 4, 2017
@fiveangle fiveangle deleted the bf2_usbcon_xonxoff branch December 6, 2017 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants