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

Start of higher level SDK #11

Closed
wants to merge 11 commits into from

Conversation

OkGoDoIt
Copy link

@OkGoDoIt OkGoDoIt commented Jul 16, 2024

Started adding a higher level SDK framework. See the readme for examples of usage.

Frame.run_lua(string: str, await_print: bool = False) -> Optional[str] executes lua regardless of length by calling Frame.send_long_lua as necessary

Frame.evaluate(string: str) -> str evaluates lua (regardless of length) and returns the result (equivalent to wrapping it in print())

Frame.send_long_lua(string: str, await_print: bool = False) -> Optional[str] executes lua longer than the MTU by writing it to a file in chunks and then running that file.

FrameFileSystem.write_file(path: str, data: bytes, checked: bool = False) writes a file in chunks. checked adds basic checking for each chunk to ensure correct writing, which is important for reliability in my testing.

Also added FrameFileSystem.file_exists and FrameFileSystem.delete_file and a bunch of others, but there's still plenty more to go.

Also added Bluetooth.set_print_debugging(value: bool) which is effectively keeps show_me on, and is useful for debugging write_file or anywhere else where the user is not directly calling Bluetooth.send_lua().

Added several tests as well. All tests are passing consistently for me on MacOS. Failures on Ubuntu due to an incompatibility between Ubuntu's BlueZ bluetooth driver and the Bleak python bluetooth library (see hbldh/bleak#1166 (comment), although the change required is out of scope for this PR). I am getting inconsistent failures on Windows due to connection instability, which will require additional testing later.

@OkGoDoIt OkGoDoIt changed the title Add long Lua sending, start of higher level SDK Start of higher level SDK Jul 17, 2024
Copy link

@rschwabco rschwabco left a comment

Choose a reason for hiding this comment

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

Left some comments.

src/frameutils/bluetooth.py Outdated Show resolved Hide resolved
src/frameutils/bluetooth.py Outdated Show resolved Hide resolved
src/frameutils/bluetooth.py Outdated Show resolved Hide resolved
src/frameutils/bluetooth.py Show resolved Hide resolved
src/frameutils/bluetooth.py Outdated Show resolved Hide resolved
src/frameutils/bluetooth.py Outdated Show resolved Hide resolved
src/frameutils/camera.py Outdated Show resolved Hide resolved
Comment on lines +32 to +33
await self.bluetooth.connect()
await self.inject_all_library_functions()

Choose a reason for hiding this comment

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

should probably handle exceptions

Copy link
Author

Choose a reason for hiding this comment

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

Are you sure? Seems best to let the exceptions bubble up since they are descriptive of the issue and we don't want to continue if we were unable to connect.

@OkGoDoIt
Copy link
Author

OkGoDoIt commented Jul 24, 2024

Thank you for the feedback. I have fixed all the concerns you both brought up, except for trapping exceptions in

async def ensure_connected(self) -> None:
"""Ensure the Frame is connected, establishing a connection if not"""
if not self.bluetooth.is_connected():
await self.bluetooth.connect()
await self.inject_all_library_functions()

The exceptions that may bubble up include when the device can't be found, where it's in a bad state and needs to be re-paired, etc. These are descriptive and not possible to work around so it seems best to let them bubble up. Do you have any ideas for how to better handle this?

Also I added display.py last night, so I'm not sure if you saw that in your review. Could you take a look if possible?

@OkGoDoIt
Copy link
Author

Per discussions with @siliconwitch, I published my Python SDK as a separate package leaving this existing frame-utilities-for-python package untouched. This allows him to use this original package for lower-level internal processes without any risk of me breaking the firmware build process, for example. Currently the new SDK is published to my personal Github account and personal PyPI account since I haven't yet been given access to anything official for @brilliantlabsAR, but obviously it would be better to put those under the Brilliant Labs accounts as soon as possible.

Here's the new frame-sdk-python package links:
Github: https://github.com/OkGoDoIt/frame-sdk-python
PyPI: https://pypi.org/project/frame-sdk/

@OkGoDoIt OkGoDoIt closed this Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants