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

Documented read_until function #356

Merged
merged 1 commit into from
Jun 18, 2018
Merged

Documented read_until function #356

merged 1 commit into from
Jun 18, 2018

Conversation

jdpatt
Copy link
Contributor

@jdpatt jdpatt commented Jun 17, 2018

I end up writing my own flavor of read_until every time to line up with telnetlib. Didn't know it was apart of pyserial already! Documented the function. I also tweaked the parameter name to match telnetlib but it is not affecting anything if that is left as terminator.

@@ -649,19 +649,19 @@ def read_all(self):
"""
return self.read(self.in_waiting)

def read_until(self, terminator=LF, size=None):
def read_until(self, expected=LF, size=None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renaming the parameter may break some code, but it makes sense to align the name with the function from telnetlib

Copy link

@bgottula bgottula Mar 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirming that this broke some code. Does this project not use semantic versioning? This rolled out with version 3.5b0 if I follow the history correctly, yet it included this API breaking change. The changelog lumps this in with "doc updates" but that is not quite accurate in this case.

@zsquareplusc zsquareplusc merged commit 826533f into pyserial:master Jun 18, 2018
@zsquareplusc
Copy link
Member

Thank you.

@jdpatt
Copy link
Contributor Author

jdpatt commented Jun 19, 2018

Looks like I should have escaped the '\'; Just noticed on read the docs that it rendered as (‘n’ by default) instead of ('\n’ by default) Thanks for the awesome package!

bgottula added a commit to seeing-things/point that referenced this pull request Mar 3, 2021
The pySerial project made a change to the parameter name for the
`read_until()` method in version 3.5. Since this project apparently does
not follow semantic versioning (made an API-breaking change without
incrementing the major version number) I have to restrict the versions
of this package that are allowed.

See also pyserial/pyserial#356 where the
possibility of breaking code was even noticed by the reviewer but then
decided to accept the pull request anyway. Why?? :(
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.

None yet

3 participants