chiefwigms / picobrew_pico

MIT License
149 stars 63 forks source link

Support for Tilt Hydrometer 1 #261

Closed cgalpin closed 3 years ago

cgalpin commented 3 years ago

This pull request is a complete implementation of Tilt support using https://github.com/atlefren/pytilt to provide the data and can be used as-is. I wanted to get this merge request in for review and to make sure there were no concerns/issues. I do not have a newer Tilt, so not sure what differences there might be for those.

I would like to discuss options for next steps to further integrate this into picobrew_pico, since right now you have to setup pytilt yourself, as well as the raspberry pi image doesn't support bluetooth right now, and would need it regardless of the approach taken to further integrate it into the image.

So options, as I see it are

  1. Add bluetooth support to the raspberry pi image (needed either way). I am not sure exactly what's different about this image from a standard raspberry pi image, but just installing the needed rpms did not work for me. I will research further.
  2. Include pytilt in the image, configured to use http://localhost/API/tilt for the url, setup as a service
  3. Include the bluetooth code as part of the server - I do not know if nginx supports running python code as a service/daemon, but if not, this won't work, because of the overhead of the bluetooth setup it needs to run as a daemon

Thoughts?

This is related to https://github.com/chiefwigms/picobrew_pico/issues/165

chiefwigms commented 3 years ago

Regarding bluetooth, I explicitly disabled it - it's a one line change in the image script, so don't go looking to far 🤣

cgalpin commented 3 years ago

Ha. Good thing I did not spend too much time on it. I did fire up another pi with a basic image to run pytilt on for my initial use/testing once i saw bluetooth wasn't working and then just used it, but should have looked closer before.

Ok, let me know what you want to do, but it should be easy to include pytilt in the image then, running as a daemon. I had to make no modifications and all it needs is an environment variable PYTILT_URL set (or set in pytilt.service if setting up as a service. There is another variable PYTILT_KEY which is sent as a http header X-PYTILT-KEY but I don't think it's needed.

cgalpin commented 3 years ago

Oh I forgot to add these screenshots

Live:

tilt_live

History:

tilt_history
tmack8001 commented 3 years ago
  1. Include the bluetooth code as part of the server - I do not know if nginx supports running python code as a service/daemon, but if not, this won't work, because of the overhead of the bluetooth setup it needs to run as a daemon

Nginx is simply a reverse proxy in front of the webserver. Isn't needed for the bluetooth code for receiving the Tilt data. I'd say best approach would be to setup pytilt in the image and potentially either always running it as a separate daemon directly (wasteful for those without tilts) or only run when a tilt_monitoring: True is specified in the server config.yaml (along with a toggle somewhere in the devices or setup/config UI).

Regardless we will need to enable bluetooth (either for everyone, or only when toggled on).

cgalpin commented 3 years ago

Haha, i should have realized that since for development i am not running nginx. lol.

Yes I like the idea of driving it off of config.yaml and a way to enable/disable it from the UI so it's only running when desired. I'll do that.

I'll also see if I can integrate this into the webserver directly though - there is no need to have it running as a separate daemon i think, and if I have to modify it to enable/disable using our config.yaml, we lose the benefit of using it unchanged. managing it will be easier in process than as a separate one.

I don't see any harm in enabling bluetooth for all.

tmack8001 commented 3 years ago

Sounds good to me @cgalpin bundled into the Picobrew server code or as a separate process is fine by me. The benefit of direct referencing within code is then it is built in for those not running on a prebuilt RaspberryPi image (assuming they have bluetooth enabled device).

Don't think we need a config setting for it unless we want to have a way to toggle off bluetooth as is the case now.

chiefwigms commented 3 years ago

I'm fine with enabling bluetooth by default in the image - originally i was just trying to minimize any added RF

cgalpin commented 3 years ago

I'm fine with enabling bluetooth by default in the image - originally i was just trying to minimize any added RF

I still like the idea of being able to disable it completely if for some reason you want to. I have not added the check yet, but I will.

I have another issue that concerns me - at least on my mac, i wasn't able to install pyBluez properly. It's failing on some dependency on a module called _lightblue which fails to build when i install pyBluez. If it's just me, then no biggie, but I don't want to break picobrew_pico on macs! I'll commit what I have for now, but if anyone with a mac wants to try install pyBluez and see if it installs cleanly, please do, and let me know what you get

sudo python3 -m pip install pyBluez

cgalpin commented 3 years ago

I have not tested building an image for the raspberry pi to verify those changes, but I will do so at some point as well.

tmack8001 commented 3 years ago

@cgalpin I could try and see what happens. Are you using a dedicated virtualenv for this project (eg ./.env) or the global system wide python environment?

cgalpin commented 3 years ago

Thanks trevor. I am new to python so not even sure but I guess global :)

I just open a shell and run sudo python3 xyz for everything. I am on OS X 11.2, have python 3.8, and Xcode 12.4. Her his what I get

Charless-MacBook-Pro:picobrew_pico cgalpin$ sudo python3 -m pip install pyBluez
WARNING: The directory '/Users/cgalpin/Library/Caches/pip' or its parent directory is not owned or is not writable by the current user. The cache has been disabled. Check the permissions and owner of that directory. If executing pip with sudo, you may want sudo's -H flag.
Collecting pyBluez
  Downloading PyBluez-0.23.tar.gz (97 kB)
     |████████████████████████████████| 97 kB 4.3 MB/s 
Requirement already satisfied: pyobjc-core>=6 in /usr/local/lib/python3.8/site-packages (from pyBluez) (7.1)
Requirement already satisfied: pyobjc-framework-Cocoa>=6 in /usr/local/lib/python3.8/site-packages (from pyBluez) (7.1)
Building wheels for collected packages: pyBluez
  Building wheel for pyBluez (setup.py) ... error
  ERROR: Command errored out with exit status 1:
   command: /usr/local/opt/python@3.8/bin/python3.8 -u -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'/private/tmp/pip-install-7go665sd/pyBluez/setup.py'"'"'; __file__='"'"'/private/tmp/pip-install-7go665sd/pyBluez/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' bdist_wheel -d /private/tmp/pip-wheel-gdxhzp4_
       cwd: /private/tmp/pip-install-7go665sd/pyBluez/
  Complete output (16 lines):
  Command line invocation:
      /Applications/Xcode.app/Contents/Developer/usr/bin/xcodebuild install -project macos/LightAquaBlue/LightAquaBlue.xcodeproj -scheme LightAquaBlue DSTROOT=/private/tmp/pip-install-7go665sd/pyBluez/macos INSTALL_PATH=/ DEPLOYMENT_LOCATION=YES

  Build settings from command line:
      DEPLOYMENT_LOCATION = YES
      DSTROOT = /private/tmp/pip-install-7go665sd/pyBluez/macos
      INSTALL_PATH = /

  xcodebuild: error: 'macos/LightAquaBlue/LightAquaBlue.xcodeproj' does not exist.
  Traceback (most recent call last):
    File "<string>", line 1, in <module>
    File "/private/tmp/pip-install-7go665sd/pyBluez/setup.py", line 79, in <module>
      subprocess.check_call([
    File "/usr/local/Cellar/python@3.8/3.8.3_2/Frameworks/Python.framework/Versions/3.8/lib/python3.8/subprocess.py", line 364, in check_call
      raise CalledProcessError(retcode, cmd)
  subprocess.CalledProcessError: Command '['xcodebuild', 'install', '-project', 'macos/LightAquaBlue/LightAquaBlue.xcodeproj', '-scheme', 'LightAquaBlue', 'DSTROOT=/private/tmp/pip-install-7go665sd/pyBluez/macos', 'INSTALL_PATH=/', 'DEPLOYMENT_LOCATION=YES']' returned non-zero exit status 66.
  ----------------------------------------
  ERROR: Failed building wheel for pyBluez
  Running setup.py clean for pyBluez
Failed to build pyBluez
Installing collected packages: pyBluez
    Running setup.py install for pyBluez ... done
Successfully installed pyBluez-0.23

Charless-MacBook-Pro:picobrew_pico cgalpin$ sudo python3 server.py 0.0.0.0 8080
Password:
cat: /etc/os-release: No such file or directory
Traceback (most recent call last):
  File "server.py", line 16, in <module>
    app = create_app(debug=True)
  File "/Users/cgalpin/Documents/OpenSource/picobrew_pico/app/__init__.py", line 29, in create_app
    from .main.config import MachineType
  File "/Users/cgalpin/Documents/OpenSource/picobrew_pico/app/main/__init__.py", line 6, in <module>
    from . import (routes_frontend, routes_server, routes_devices, routes_support, 
  File "/Users/cgalpin/Documents/OpenSource/picobrew_pico/app/main/routes_tilt_api.py", line 6, in <module>
    from .tilt import process_tilt_data
  File "/Users/cgalpin/Documents/OpenSource/picobrew_pico/app/main/tilt.py", line 5, in <module>
    import bluetooth._bluetooth as bluez
  File "/usr/local/lib/python3.8/site-packages/bluetooth/__init__.py", line 47, in <module>
    from .macos import *
  File "/usr/local/lib/python3.8/site-packages/bluetooth/macos.py", line 1, in <module>
    import lightblue
  File "/usr/local/lib/python3.8/site-packages/lightblue/__init__.py", line 160, in <module>
    from _lightblue import *
ModuleNotFoundError: No module named '_lightblue'
Charless-MacBook-Pro:picobrew_pico cgalpin$ 
sickchilly commented 3 years ago

Bluez is meant for general Linux platforms and will not compile or work on Macs. If you want to develop for Bluetooth on Macs, you'll need to consult Apple's developer documentation.

As for pybluez specifically, it seems most repositories are horribly out of date. Someone reported success with the GitHub version: https://github.com/pybluez/pybluez

cgalpin commented 3 years ago

mmh, thats not good news. I installed PyBluez from github (0.30) and that install runs without errors. But I still get the error

ModuleNotFoundError: No module named '_lightblue'

At the risk of angering Carol Baskin, this dude appears to have fixed it, but I can't get that to work.

https://github.com/tigerking/pybluez

I'm not a python or objective C guy but muddled through it. It appears his approach builds a wheel, and I had to make a code change to get it to compile, and at that point i got past this _lightblue problem. But....

This 0.30 version differs somehow which I don't really understand. In 0.23 the import looks like

import bluetooth._bluetooth as bluez

which no longer works in 0.30. I honestly don't know where the _bluetooth comes from, but in 0.30 you can't import it that way. Any idea how it would need to change? The blues.py in that package does do a import bluetooth._bluetooth as _bt so i am confounded!

cgalpin commented 3 years ago

BTW, I'm personally ok with this not working on a mac as long as it doesn't stop you installing and running picobrew_pico on a mac. Right now this is a show stopper. I guess I could wrap the import with an OS check for mac, but eew.

cgalpin commented 3 years ago

In an effort to get this merged I have made a change to disable the bluetooth feature on macOS, so everything else continues to work. Anyone wanting to run picobrew_pico on a mac and use a tilt, can run pytilt on some other machine.

Tesed on mac and raspberry pi (not the image stuff).

tmack8001 commented 3 years ago

As I understand it @cgalpin _bluetooth is a private module within the bluetooth package. Specifically looking at sources this is a private module with only support on linux whereas lightblue is the module for interacting with macos.

I would highly encourage not using the low level bluetooth API provided by bluetooth._bluetooth unless we want to deal with all the different platform specifics doing so brings to the table. Does bluez not provide the tough points with the Bluetooth protocol that we need to support scanning and reading the transmissions from Tilt devices?

cgalpin commented 3 years ago

I did not look closely at pytilt and mistakenly made the assumption that it would be portable. My bad. It seemed a perfect way to get this feature added given it was a working solution already.

I can look into other libraries, but as-is this should work on the pi and I assume windows, so has value since I suspect most people who'd use it will just run all of this on a pi. The first one I will look at is https://github.com/hbldh/bleak

Limited support is better than no support, right? We can upgrade/enhance later, no?

tmack8001 commented 3 years ago

@cgalpin seems the github action type can't install from source the pybluez dependency. This is blocking the CI of this PR. The packaging of pybluez is weird to me since it has to be built by source, but guess that is due to the different transitive dependencies depending on the platform you are installing from.

We will likely need to add additional dependencies to the github action environment and then likely similarly will need to be done for building the raspberrypi image.

cgalpin commented 3 years ago

Ok, please don't bother for now then. Once I get bleak working, which i'm guessing will have the same issue, we can address then.

tmack8001 commented 3 years ago

Ok, are you close or need a hand getting the encoding of the protocol. BTW emailing baronbrew has in the past been super easy to get information on the engineering protocol of the Tilt though my asks there were more on the API protocol of the tiltpi interface and related apps than the raw Bluetooth signal encoding. Though I don't see that as proprietary and they would share it anyways.

tmack8001 commented 3 years ago

Believe from what I've looked at in the past the data in the beacon broadcast is to be an array of CVS data points.

MAC,SG,TEMP
MAC,SG,TEMP

SG would need to divide by 1000

For any ABV calculations we can use this:

abv = (original_gravity - specific_gravity) * 131.25;
cgalpin commented 3 years ago

No, I'm good (although packing it in for today).

It's an iBeacon so the format is published - it's not csv. I'm just slow and not used to python doesn't help :)

I've figured out the decoding for the uuid, temp, and gravity and the signal strength is there too so I may add that. But I need to turn my test into workable code and test. I should just wrap it up this evening, but my wife doesn't share this as an interest :)

If not tomorrow, I'll definitely wrap this up over the weekend.

cgalpin commented 3 years ago

Documentation updated. The only outstanding issues I am aware of are

  1. fix the github actions to include bluez support (@tmack8001 is looking into this)
  2. test building a raspberry pi image with this supported
  3. how to enable bluetooth support for existing installs

1 is a show stopper, but I don't think #2 and #3 are.

2 i can look into. I see the setup is documented in scripts/pi/_readme.txt, but I am not sure what to do after that :) I'll research.

3 @tmack8001 i think you mentioned you wanted to look into #3, but if not, let me know. It shouldn't be too hard to run system commands to do this, but for the short term, we can document the steps for power users/early adopters to enable bluetooth.

I think the restart_server function in routes_server.py can be updated to check for the existence of an upgrade shell script which can be fairly sophisticated. That script could do all the checks necessary to make sure it's running on a pi, then check for bluetooth support, enable in config.yaml, etc. I think we could even force an OS reboot there too to fully enable bluetooth automatically. Again, let me know if you want me to do this.

tmack8001 commented 3 years ago

@cgalpin I fixed the bluez issue, https://github.com/cgalpin/picobrew_pico/pull/3 so once that gets in I'm good with a merge. We can figure out the "no touch" upgrade path later.

chiefwigms commented 3 years ago

i'm good with this - @tmack8001 merge whenever you're good