Skip to content

Devices.py improvements#122

Open
dhouck wants to merge 4 commits intoNorth-Western-Development:1.20.1from
dhouck:python-devices-improvements
Open

Devices.py improvements#122
dhouck wants to merge 4 commits intoNorth-Western-Development:1.20.1from
dhouck:python-devices-improvements

Conversation

@dhouck
Copy link

@dhouck dhouck commented Feb 16, 2026

There is currently half-working functionality for device subscriptions and events, where devices can push data to computers instead of needing method calls. I am working on fixing that up, and in the process found some issues with the Python library.

In particular:

  • Making Device.methods a property is just a quality of life improvement; it was already available, but only if you happened to call __str__ first.
  • The bugfixes for DeviceBus._read_message and the added helper functions are things I found while implementing events. I did test without any of my event changes, but they should rarely matter in the current implementation.

I can read Lua well enough to tell that the Lua library seems to have the same issues, but Iʼm not sure I could fix it.

@dhouck
Copy link
Author

dhouck commented Feb 18, 2026

I need to fix that last commit, json.loads on a bytes object is not fully reliable in micropython so I need to make a quick change. More details: this is only if using the micropython-lib/unix-ffi version of the json module, which is on the system but not loaded by default; it has a bug where it will not always properly decode a bytes object. However, that version is also unusably buggy in a different way. If Buildroot ever updates the micropython libs (which Iʼm making another MR about tonight or tomorrow), I expect itʼll either still be broken or have my fix for that.

However, both that json lib and CPython raise an exception if there is an extra null byte at the end, so I need to fix this anyway to not include the delimiter at the end of the message if I want it to be compatible with CPython not just MicroPython.

@dhouck dhouck force-pushed the python-devices-improvements branch from d349cba to 343f9d7 Compare February 18, 2026 06:59
@dhouck
Copy link
Author

dhouck commented Feb 18, 2026

And those removed four characters should make it work with other (working) JSON modules, including the CPython one (not tested) and the micropython-lib/unix-ffi one if they fix re and merge my PR. However the only one I was fully able to check with is the builtin micropython json lib weʼre currently using.

@Un1q32
Copy link

Un1q32 commented Feb 20, 2026

idk python so i cant properly review this

@dhouck
Copy link
Author

dhouck commented Feb 21, 2026

It is not very important until I make the PR that can turn on subscriptions and events; the chance of any of the issues except the Device.methods one happening in the wild is pretty slim without that. Hopefully Iʼll make at least a draft PR for that tomorrow; Iʼve gone down a rabbit hole trying to fix the last known issue but I have a hacky mostly-working solution, and thoughts about better solutions.

Even then, unless someone actually uses subscriptions the issues probably wonʼt come up, and this doesnʼt make subscriptions easy to use yet, just possible.

dhouck added 4 commits March 2, 2026 20:19
This way it can be used without calling str() first
Previously the DeviceBus._read_message function had bugs with
consecutive empty messages (should rarely happen, but should still be
handled appropriately) or with two delimeters at the end of a buffer
(happens when one message ends almost at the end of the buffer and
another begins at the last byte of the buffer, which should be rare but
is more common with subscriptions).  This refactor should be more
robust, and I believe also faster but I haven't benchmarked it.

In the process, make the entire function work on bytearrays instead of
converting to string early; this shouldn't make a difference with the
JSON being all ASCII, but can if the JSON is ever serialized in UTF-8
instead of using escape sequences.
This function is used in a couple places and this makes it clearer what
the poll method means
These can be caught the same way, but can also be caught more
specifically, and can carry more useful detail.
@dhouck dhouck force-pushed the python-devices-improvements branch from 343f9d7 to 01357d4 Compare March 7, 2026 06:03
@dhouck
Copy link
Author

dhouck commented Mar 7, 2026

I added just rebased on the latest upstream, and added two more small commits. One is minor refactoring, and the other is using custom exception types instead of base Exception for the errors that are raised.

I had (incorrectly, it looks like, but Iʼm not yet certain) thought the refactoring would help with the events support in the library; I kept it because I think having that factored out is still better. The custom exception types just make issues easier to deal with; this should improve things for people using the library but also make it easier to spot issues when Iʼm doing library development.

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