Skip to content

Update sn-bt-setup-peripheral.py#6

Open
eljpsm wants to merge 5 commits into
SolarNetwork:masterfrom
eljpsm:update-sn-bt-setup-peripheral
Open

Update sn-bt-setup-peripheral.py#6
eljpsm wants to merge 5 commits into
SolarNetwork:masterfrom
eljpsm:update-sn-bt-setup-peripheral

Conversation

@eljpsm
Copy link
Copy Markdown

@eljpsm eljpsm commented Jun 3, 2026

Updates the sn-bt-setup-peripheral.py after renewed testing for Bluetooth connectivity.

This presents some breaking changes:

  • The TX characteristic is now notify instead of read.
  • Some sentinel bytes have shifted: \x07 is kept, but both \x26 and \x00 have been removed. \x18 has been added for session tear down, server pull down, OSError, etc.
  • The request and response is now streamed rather than synchronous.

Some extra environment variables are also read for debugging at the moment, but could be wired in the future for better observability in the node if required.

I've tried to rearrange the format of the file a bit to also keep it in style with the reference BlueZ file examples, though I admit it still looks a little messy. Most of this is a result of real testing with Bluetooth devices, hence some of the defensive code additions. I've tried to document them to keep them a bit more readable.

@msqr
Copy link
Copy Markdown
Contributor

msqr commented Jun 4, 2026

Looking reasonable to me (although I cannot really judge the Python code itself). I would ask if you could capture some of the documentation on the supported environment variables into the package README file? We could use that info to perhaps tweak the systemd service to read in an optional environment file.

@msqr
Copy link
Copy Markdown
Contributor

msqr commented Jun 4, 2026

Looking good with the README and Makefile updates, thanks. Other packages make it super easy for users to define service environment variable files in the /etc/solarnode directory... how about you tweak the [Service] section of the service unit to support that, e.g. add

EnvironmentFile=-/etc/solarnode/bluetooth-setup.env

For example we could tweak the logging then by creating that file with

SN_BT_SETUP_PERIPHERAL_LOG_LEVEL=debug

Comment thread solarnode-bluetooth-setup/debian/README.md Outdated
Comment thread solarnode-bluetooth-setup/debian/README.md Outdated
Copy link
Copy Markdown
Contributor

@msqr msqr left a comment

Choose a reason for hiding this comment

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

These changes look good. If you'd like them merged, you could drop the Draft status and let me know.

@eljpsm eljpsm marked this pull request as ready for review June 7, 2026 20:42
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