Skip to content

Fix Backwards Logic of 'wait_for_response' In #3005 #3006

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 1 commit into from
Jun 6, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions tools/cpboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ def reset(self):
self.write(b"\r" + REPL.CHAR_CTRL_B) # enter or reset friendly repl
data = self.read_until(b">>> ")

def execute(self, code, timeout=10, wait_for_response=False):
def execute(self, code, timeout=10, wait_for_response=True):
Copy link

Choose a reason for hiding this comment

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

Thanks. It might be a bit better to def execute(self, code, timeout=10, *, wait_for_response=True): so that any old code that called execute(code, timeout, True) intending to get async behvior will be a runtime error, instead of turning into a call with wait_for_response=True. If the only users are in the circuitpython source tree, though, this scarcely matters.

Copy link
Author

@sommersoft sommersoft Jun 4, 2020

Choose a reason for hiding this comment

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

This is a very excellent point, and I don't consider it a nit-pick at all.

I don't expect many people are using this. I point to it as an example how to interact with boards via a PC, but that's about it.

A lot of the semantics are so that it remains py2/py3 compatible, from what I understand (its base is MicroPython's tools/pyboard.py). I'll likely give it more scrubs for a multitude of reasons. One of those scrubs will include removing Python2 use; not worth using at this point.

self.read() # Throw away

self.write(REPL.CHAR_CTRL_A)
Expand All @@ -136,7 +136,7 @@ def execute(self, code, timeout=10, wait_for_response=False):
self.write(code)

self.write(REPL.CHAR_CTRL_D)
if wait_for_response:
if not wait_for_response:
return b"", b""
self.read_until(b"OK")

Expand Down Expand Up @@ -450,7 +450,7 @@ def close(self):
self.serial.close()
self.serial = None

def exec(self, command, timeout=10, wait_for_response=False):
def exec(self, command, timeout=10, wait_for_response=True):
with self.repl as repl:
try:
output, error = repl.execute(
Expand Down Expand Up @@ -483,7 +483,7 @@ def _reset(self, mode="NORMAL"):
)
try:
self.exec(
"import microcontroller;microcontroller.reset()", wait_for_response=True
"import microcontroller;microcontroller.reset()", wait_for_response=False
)
except OSError:
pass
Expand Down