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

Properly decode UTF-8 characters comming in from serial one byte at a time. #5967

Merged
merged 1 commit into from Feb 20, 2017
Merged

Properly decode UTF-8 characters comming in from serial one byte at a time. #5967

merged 1 commit into from Feb 20, 2017

Conversation

@aknrdureegaesr
Copy link

@aknrdureegaesr aknrdureegaesr commented Feb 9, 2017

This fixes #2430.

… time.

This fixes #2430.
@facchinm
Copy link
Member

@facchinm facchinm commented Feb 10, 2017

I can see some performance penalty here with sketches writing a lot of stuff.
@PaulStoffregen, what do you think about it?

@aknrdureegaesr
Copy link
Author

@aknrdureegaesr aknrdureegaesr commented Feb 10, 2017

My gut feeling is that the raw throughput of any such code (both mine and the previous) is much higher than any serial port or any UI work can ever be.

If that helps any, I offer to provide performance tests to prove that point.

@PaulStoffregen
Copy link
Contributor

@PaulStoffregen PaulStoffregen commented Feb 10, 2017

I think testing is needed with Arduino Due and/or Teensy 3.6. Where's @ArduinoBot when we need him?

int copyNow = buf.length - next < spaceInIn ? buf.length - next : spaceInIn;
inFromSerial.put(buf, next, copyNow);
next += copyNow;
inFromSerial.flip();

This comment has been minimized.

@cmaglie

cmaglie Feb 10, 2017
Member

Why flip() is called?

This comment has been minimized.

@aknrdureegaesr

aknrdureegaesr Feb 10, 2017
Author

The buffer needs to be switched from being written to to being read from, which is what flip does.

Both clear and compact switch the buffer from having being read to being ready to accept writes.

@PaulStoffregen
Copy link
Contributor

@PaulStoffregen PaulStoffregen commented Feb 10, 2017

Ok, compiled and running it now. Seems to be ok CPU-wise. Tested only on Linux 64 bit running on a fast desktop machine.

@facchinm
Copy link
Member

@facchinm facchinm commented Feb 10, 2017

@ArduinoBot build this please.

@aknrdureegaesr
Copy link
Author

@aknrdureegaesr aknrdureegaesr commented Feb 18, 2017

What's the usual process with pull requests here in the Arduino repo? I thought, the obvious, "push a pull request and wait a few days".

But there has been nothing here for more than a week now. For that week, I've been waiting for the maintainers to move. I hope they are not simultaneously waiting for me.

Is anything missing I'm expected to provide? If so, I'm not aware of it. Please tell me.

Or what's been hindering this to be merged, if I may ask?

@matthijskooijman
Copy link
Collaborator

@matthijskooijman matthijskooijman commented Feb 18, 2017

You have the process correct, but it typically takes more than a week for a PR to be thoroughly reviewed and tested, so it can be merged. From the comments, this PR is now waiting for @cmaglie or @facchinm to find some time to have another look, so I don't think any action is required on your side.

@facchinm facchinm added this to the Release 1.8.2 milestone Feb 20, 2017
@cmaglie cmaglie merged commit ba302ee into arduino:master Feb 20, 2017
@cmaglie
Copy link
Member

@cmaglie cmaglie commented Feb 20, 2017

@aknrdureegaesr
thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

7 participants