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

[1.1.x] SERIAL_XON_XOFF not supported on USB-native AVR devices #8653

Merged

Conversation

fiveangle
Copy link
Contributor

@fiveangle fiveangle commented Dec 4, 2017

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.

@thinkyhead - can you please review this one as I may have made some incorrect assumptions when porting this fix from bugfix2.0.x where there were some changes in this area. I tried relocating the sanity checks from within MarlinSerial.h but it would break non-USB AVR, and vice versa (which is why I believe you may have left them there in the first place ?)

'Spent way too long messing with this 1.1.x port. I tested all error conditions and confirms it works as expected, but you should take a quick once over just in case it could be done better (note the relocation of USBCON check below the setting of the default buffers sizes to pick those up for both, while USBCON is taken care of in Conditionals_adv.h in bf2).

I think this just further illustrates the need to release 2.0 + 1.1.7 and leave 1.1.7 to strict bugfix-only ;)

@fiveangle fiveangle force-pushed the bf1_usbcon_xonxoff branch 3 times, most recently from d4d0a47 to 2ba79a6 Compare December 4, 2017 14:30
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.

#ifndef USBCON
Copy link
Contributor Author

@fiveangle fiveangle Dec 4, 2017

Choose a reason for hiding this comment

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

My biggest question is why all these tests can't be run even if USBCON is defined ? But it's 1.1.x so I'm not going to lose any sleep over it ;)

Copy link
Member

Choose a reason for hiding this comment

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

It looks like we're assuming those options can't be overridden on platforms that have USBCON. I'd have to reach back into the Git Blame annals to find out the reasoning.

@thinkyhead thinkyhead merged commit 860d98a into MarlinFirmware:bugfix-1.1.x Dec 4, 2017
@fiveangle fiveangle deleted the bf1_usbcon_xonxoff branch December 6, 2017 18:28
mtm88001 pushed a commit to mtm88001/Marlin that referenced this pull request Dec 9, 2017
…inFirmware#8653)

* SERIAL_XON_XOFF not supported on USB-native AVR devices

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.
damicreabox pushed a commit to damicreabox/Marlin that referenced this pull request Sep 14, 2018
…inFirmware#8653)

* SERIAL_XON_XOFF not supported on USB-native AVR devices

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.
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.

None yet

2 participants