Skip to content

Issue #46: motor.command() and CRLF #75

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 2 commits into from
May 13, 2021
Merged

Issue #46: motor.command() and CRLF #75

merged 2 commits into from
May 13, 2021

Conversation

maxlem
Copy link
Contributor

@maxlem maxlem commented May 10, 2021

The eol character is now configurable
The user can now actuvate echo feedback to see what he is typing
I also removed a duplication in two run() overload, while fixing what I think was a mistake by enforcing the use of the provided argument Stream& even if the ctor one is not null.
Lastly, I warn the user if \n is configured but \r is detected (only in user friendly mode)
All the past code should work as before (no API change required)

The eol character is now configurable
The user can now actuvate echo feedback to see what he is typing
I also removed a duplication in two run() overload, while fixing what I think was a mistake by enforcing the use of the provided argument Stream& even if the ctor one is not null.
Lastly, I warn the user if \n is configured but \r is detected (only in user friendly mode)
All the past code should work as before (no API change required)
@maxlem maxlem mentioned this pull request May 10, 2021
@askuric askuric changed the base branch from master to dev May 12, 2021 11:31
@maxlem maxlem mentioned this pull request May 12, 2021
Merged
@askuric
Copy link
Member

askuric commented May 13, 2021

I'm ok with this PR, I'm merging it now.

The user can now actuvate echo feedback to see what he is typing

I'll leave it for now, but I'd like to solve this a bit more consistently with the library API in future. Introduce a VerboseMode::echo or something like that.

I also removed a duplication in two run() overload, while fixing what I think was a mistake by enforcing the use of the provided argument Stream& even if the ctor one is not null.

This was a feature not a bug :D
So that the user can use more than one serial if he wants or that he does not have to define serial in the constructor. But there are problems with this implementation, I agree. Like the common buffer and similar.

Lastly, I warn the user if \n is configured but \r is detected (only in user friendly mode)

This is a good idea, but the text is too long for Arudino UNO. I'll shrink it a bit :D

@askuric askuric merged commit 8246fe5 into simplefoc:dev May 13, 2021
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.

3 participants