Skip to content

[MP1] Add RPMsg virtual serial protocol support (VirtIOSerial) Update proposal #1

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

Merged
merged 0 commits into from
Feb 11, 2020

Conversation

ABOSTM
Copy link

@ABOSTM ABOSTM commented Jan 31, 2020

Summary
This PR propose some update after a review of your PR:
stm32duino#766

Major update proposed:

  • rebase on master (at least master at the time I started)
  • virtio_buffer: rework to get ride of read_tmp
    Note: I stick to Arduino official definition:
    https://www.arduino.cc/reference/en/language/functions/communication/serial/peek/
    "That is, successive calls to peek() will return the same character, as will the next call to read()."
  • Manage virtio_buffer as ring buffer with rollover
  • VirtIOSerial multi instances support.
    Even if not currently used, it is better to have muti instance support for this C++ class.

I tested those changes with the sketch you proposed, and it work fine.
I just change this line to receive in minicom the same ASCII characters that has been send.
    Serial.print( (char) Serial.read());

@kbumsik
Copy link
Owner

kbumsik commented Feb 10, 2020

I will accept this PR for now to move the discussion to the original PR. I don't know how to accept this PR in Github interface though. Is it okay for you if I just reset my branch to yours and close this PR?

@ABOSTM
Copy link
Author

ABOSTM commented Feb 10, 2020

You should be able to approve and then merge the PR, which will automatically uptade the original PR

@kbumsik
Copy link
Owner

kbumsik commented Feb 11, 2020

image
I mean it says like this...lol
And also when I tried using it on Github Desktop as Github suggests (I'm new to this GUI, I only use git cli...) it shows a lot of conflicts. Maybe Github PR doesn't work well with an upstream rebased branch like your PR.
I'm gonna hard reset my branch to your branch for now....let's see what happens.

@kbumsik kbumsik merged commit acc2416 into kbumsik:stm32mp1 Feb 11, 2020
@kbumsik
Copy link
Owner

kbumsik commented Feb 11, 2020

Github recognizes it as merged automatically even if I force push commit using git command line. Smart.

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

Successfully merging this pull request may close these issues.

2 participants