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

Handle encoding error in mks_g_series driver #19

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ScottSoren
Copy link
Contributor

In PyExpLabSys/drivers/mks_g_series.py, MksGSeries.com() now handles the error rather than crashing when the flow controller sends it a response it can't decode to ascii.
Also fixed the name of the MksGSeries class in macines/rasppi89/socket_server.py and made that script python3-compatible by converting a dict.keys() to list before indexing.

In PyExpLabSys/drivers/mks_g_series.py, MksGSeries.com() now handles the error rather than crashing when the flow controller sends it a response it can't decode to ascii.
Also fixed the name of the MksGSeries class in macines/rasppi89/socket_server.py and made that script python3-compatible by converting a dict.keys() to list before indexing.
@@ -35,7 +38,10 @@ def comm(self, command, addr):
self.ser.write(com_string)
time.sleep(0.1)
reply = self.ser.read(self.ser.inWaiting())
reply = reply.decode()
try: #some MFC communication gives UnicodeDecodeError
reply = reply.decode()
Copy link
Member

Choose a reason for hiding this comment

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

Not related to this PR, but the default behavior of decode() without arguments has changed from 'ascii' to 'utf-8'. Most likely, the intention is that it should be ascii and getting bytes that cannot be decoded as ascii is a communications error. So trying to decode as utf-8 might in principle allow scrambled message to be decoded and the fault will appear somewhere else. I suggest to explicitely add the 'ascii' argument to decode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changing from ascii to utf-8 was actually the first thing I tried. I changed, a few lines above in the same function,
com_string = com_string.encode('ascii')
to
com_string = com_string.encode('utf-8')
Gave the same error message. But of course in hindsight that makes sense, after I figured out it was the returning message that gave the error.
Strangely, though, this wasn't a problem before updating the rest of the drive to Robert's version. It's not a py2 vs py3 thing since I tried both...

I'll try your suggestion, and make a new pull request if it works, next time I have down time between experiments at my setup. It's a kind of tedious process to edit in on the rasppi (a requirement so that I can test it) and then copy it over to my fork to pull request it, but I think it's worth it for now so you can suggest changes just like this.

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