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

[Discussion] async version #63

Open
tdamsma opened this issue Jun 5, 2020 · 10 comments
Open

[Discussion] async version #63

tdamsma opened this issue Jun 5, 2020 · 10 comments

Comments

@tdamsma
Copy link

tdamsma commented Jun 5, 2020

First of all thanks for the wonderful library and inspiring examples!

As I was fiddling with the (asynchronous) bleak library I realized that some of the commands in pylgbst are blocking even though that is technically not necessary. As far as I can see, it is not possible to e.g. blink a light and move a motor at the same time.

As the bleak library uses asyncio, I hacked together an async/await proof-pf-concept that allows much better control over simultaneous operations. Have you considered a fully async version? And/or would you be interested to work on an async fork of this library?

@undera
Copy link
Owner

undera commented Jun 5, 2020

I am myself ok with async concept, since I am an experienced developer. But I believe most of the library audience is not that advanced, and blocking style is easier to learn and use for non-sophisticated users.

If you'll look into library's internals, you'll see that it's multi-threaded and supports parallel operations, just it does that with classic threads (or sensor subscriptions). Also, there are non-blocking modes in Boost protocol itself, which library might not fully support at the moment.

Finally, there is already an async-style lib for this: https://github.com/virantha/bricknil . I don't see a point in duplicating it. I'd rather keep alternative approach for that part of community that prefers it. Just for the sake of offering a choice.

What would really help current project, is an implementation of bleak-based connection driver. There are already couple of PRs on that, but nothing that fully works.

@tdamsma
Copy link
Author

tdamsma commented Jun 5, 2020

Thanks for detailing your considerations, makes perfect sense now. I didn't know about bricknil, will check that out!

Perhaps a pointer to bricknil would be nice, as your library ranks much higher when searching for "Lego Boost Python".

@naelcm
Copy link

naelcm commented Jun 10, 2020

I tried bricknil and found it to be flakey and hard to use. The synchronous design suits me better. However it would be nice to be able to command a motor to make a new move before the current move has finished, with the new move replacing the current move. For example you tell a car to steer right, and then realise it should have been left. As it stands I can't "replace" the steer right command until the motor has finsihed moving. Maybe this already supported somehow, I will look into the use_profile parameter.

@LasseD
Copy link

LasseD commented Jun 29, 2020

I am also trying to make it work with bleak since bleak is the only library mentioned which works on MacOS according to the main README. Unfortunately the latest version of bleak doesn't work with pylgbst. It seems like there have been some API changes since bleak was introduced to this library, since there are some async function which are called without await (thus resulting in the functions not being called). Some async methods are called in constructors, so they require rather large changes to the program flow (factory pattern, or calling "follow up methods upon creation", etc.)

Which PR seems most promising?

PS. bricknil currently doesn't work with bleak either. This is most likely also due to changes in bleak 0.6.4. It gives me the "Unknown device with id 66 being attached..." issue, which is currently unresolved.

@undera
Copy link
Owner

undera commented Jun 30, 2020

I have tested Bleak connection type on my Linux and it did work fine for demo.py script. Maybe the issue with MacOS is platform-specific. I don't have Mac to troubleshoot this, sorry. Maybe somebody else would investigate.

@LasseD
Copy link

LasseD commented Jun 30, 2020

That's alright. Perhaps I can help here, since I have access to a MacOS device. The current versions are:

pylgbst 1.2.0
bleak 0.6.4

And this is the trace when running demo.py from a fresh clone (python3 examples/demo.py):

INFO:root:Trying get_connection_bluepy
INFO:root:Trying get_connection_bluegiga
INFO:root:Trying get_connection_gatt
INFO:root:Trying get_connection_bleak
WARNING:bleak.backends.corebluetooth.CentralManagerDelegate:CentralManagerDelegate is not compliant
INFO:comms-bleak:Discovering devices... Press green button on Hub
Exception in thread Thread-1:
Traceback (most recent call last):
File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/threading.py", line 932, in _bootstrap_inner
self.run()
File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/threading.py", line 870, in run
self._target(*self._args, **self._kwargs)
File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/site-packages/pylgbst/comms/cbleak.py", line 50, in
self._connection_thread = threading.Thread(target=lambda: asyncio.run(self._bleak_thread()))
File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/asyncio/runners.py", line 43, in run
return loop.run_until_complete(main)
File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/asyncio/base_events.py", line 616, in run_until_complete
return future.result()
File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/site-packages/pylgbst/comms/cbleak.py", line 60, in _bleak_thread
await bleak.connect(self.hub_mac, self.hub_name)
File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/site-packages/pylgbst/comms/cbleak.py", line 142, in connect
devices = await discover(timeout=10)
File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/site-packages/bleak/backends/corebluetooth/discovery.py", line 31, in discover
raise BleakError("Bluetooth device is turned off")
bleak.exc.BleakError: Bluetooth device is turned off
WARNING:hub:Got only these devices: (None, None, None, None, None, None, None)
Traceback (most recent call last):
File "examples/demo.py", line 259, in
hub = MoveHub(**parameters)
File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/site-packages/pylgbst/hub.py", line 221, in init
self._report_status()
File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/site-packages/pylgbst/hub.py", line 238, in _report_status
name = self.send(MsgHubProperties(MsgHubProperties.ADVERTISE_NAME, MsgHubProperties.UPD_REQUEST))
File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/site-packages/pylgbst/hub.py", line 71, in send
self.connection.write(self.HUB_HARDWARE_HANDLE, msgbytes)
File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/site-packages/pylgbst/comms/cbleak.py", line 96, in write
raise ConnectionError('Something went wrong, communication threads not functioning.')
ConnectionError: Something went wrong, communication threads not functioning.

From the bleak project it seems like "Bluetooth device is turned off" is a known issue: hbldh/bleak#206

@undera
Copy link
Owner

undera commented Jun 30, 2020

I have made an alternative implementation in #70 . But that does not help too much. It looks like bleak is quite unstable with its functioning.

@LasseD
Copy link

LasseD commented Jul 6, 2020

Yeah. I have not been able to make that one work either. I will try to get an MVP going with just hard coded "Discover. Connect. Send a single move command. Disconnect." If I can get this, then I will have a better comparison and starting point for debugging. Unfortunately I will at earliest have time for this next week.

@finik
Copy link
Contributor

finik commented Sep 13, 2020

I managed to make it work on a mac, see #76

@LasseD
Copy link

LasseD commented Sep 20, 2020

I have tried your fix and it works even on Catalina!

Running go_toward_light.py makes Vernie travel around until he encounters:

Exception in thread Thread-2:
Traceback (most recent call last):
File "threading.py", line 932, in _bootstrap_inner
self.run()
File "threading.py", line 870, in run
self._target(*self._args, **self._kwargs)
File "cbleak.py", line 81, in _processing
self._handler(msg[0], bytes(msg[1]))
File "hub.py", line 84, in _notify
msg = self._get_upstream_msg(data)
File "hub.py", line 103, in _get_upstream_msg
msg = msg_kind.decode(data)
File "messages.py", line 722, in decode
assert len(msg.payload) == 2, "TODO: implement multi-port feedback message"
AssertionError: TODO: implement multi-port feedback message

But that just have to be implemented. Now at least I have a starting point!

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

No branches or pull requests

5 participants