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.

Sign up for GitHub

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

Jump to bottom
Merged
askuric merged 2 commits into from
May 13, 2021
Merged

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

askuric 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)

@maxlem-neuralium
Issue simplefoc#46: motor.command() and CRLF
ce52535 
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
Closed
@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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@maxlem @askuric @maxlem-neuralium