Skip to content

Initial implementation of 9bit data support #6443

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

svatoun
Copy link

@svatoun svatoun commented Jun 30, 2017

Pull request to fix issue #6442.

Initial implementation of conditional 9bit support. Since 9bit transfers are rather niche, the implementation does not (almost) add any overhead for standard usage, buffer expansion and more complicated processing is guarded by SERIAL9 macro. Users must put -DSERIAL9 macro into their platform.txt in order to use the support - I didn't found other suitable way. Should be easier in the future if support for -D defines is added to the arduino IDE.

@facchinm facchinm added Architecture: AVR Applies only to the AVR microcontrollers (Uno, etc.) Component: HardwareSerial The hardware serial functionality of the core libraries feature request A request to make an enhancement (not a bug fix) labels Jul 3, 2017
@svatoun
Copy link
Author

svatoun commented Aug 24, 2017

any news / progress / estimate when the PR could be reviewed(or rejected) ?

@mikaelpatel
Copy link

What are the performance implications? How much does the effective baud-rate drop due to the additional handling of configuration? At lower baud-rates there is no impact but I guess that above approx 115Kbps the drop will start. A benchmark is required.

As 9-bit mode is not a very common use case for Serial I would suggest considering a Serial sub-class. Also typical 9-bit mode usage is to signal the start and end of frames. This could be implemented without modifying the current Serial API.

Cheers! Mikael

@PaulStoffregen
Copy link
Contributor

If this is merged (a pretty big "if"), I would like to suggest a longer, more descriptive define than SERIAL9. Perhaps SERIAL_9BIT_SUPPORT could work?

@basprins
Copy link

Any news on this? Multidrop bus is a 9bit uart protocol. I could sure benefit if this branch would ever be merged!

@svatoun
Copy link
Author

svatoun commented Jul 31, 2019

@basprins I admit that I was lazy delivering the requested benchmarks; partially because given the nature of changes, the performance impact cannot be too high: one conditional jump based on register data operation, one IO write per-byte (not bit!). Similar for read.

IMHO worse effect is doubling the rx/tx buffer because of unsigned char to unsigned short change. Can be seen as bad on smaller Arduinos (I use Nanos ;)). Can be probably changed at the expense of some shift/and to use a bitfield for 9th bits - pls. give your opinions on whether you see that as worth the increased complexity.

Also note that if the guard macro is not defined, the change simply will not exist at all, leaving original code as it was - performance impact zero.

@mikaelpatel But now I have two A-Mega boards available, so I can make some measurement. I assume that we need at least to: prove that 16MHz is able to tx / rx 115kbps. That's trivial - but do you have some suggestion to actually measure 'net consumed time' for (say) the write operation ? Sequential write or read operations are limited by UART speed (when it becomes avialable again) - how to measure the really tiny CPU time that one write opreation takes ?
Thanks for the advice.

Re.: API change: good point; the write9 methods can be removed. Thanks, will change.

@PaulStoffregen: good idea; will do on the very next change, the change is going to be further considered for acceptance

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@mikaelpatel
Copy link

@svatoun After the latest @CLAassistant notice I revisited this pull request. I believe that if the purpose of 9-bit mode is to allow multi-drop protocols with the 9th bit for indicating address or data this could be done by simply adding a sendAddress(uint8_t addr) function to the HardwareSerial API. This function would set/reset the 9th bit accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Architecture: AVR Applies only to the AVR microcontrollers (Uno, etc.) Component: HardwareSerial The hardware serial functionality of the core libraries feature request A request to make an enhancement (not a bug fix)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants