crownstone / bluenet

Bluenet is the in-house firmware on Crownstone hardware. Functions: switching, dimming, energy monitoring, presence detection, indoor localization, switchcraft.
https://crownstone.rocks
91 stars 62 forks source link

Recognize if we're in a python virtual env and report to the user #138

Closed mrquincle closed 2 years ago

mrquincle commented 2 years ago

The recommended way to install python tools is described at https://github.com/crownstone/crownstone-lib-python-core/blob/master/docs/TUTORIAL_VENV_SETUP.md.

The parent https://github.com/crownstone/bluenet/blob/master/CMakeLists.txt file removes some of the burden of a first-time developer of the bluenet code.

For example, when -DDOWNLOAD_NRFUTIL=ON there's an nrfutil target created through the use of ExternalProject_Add that uses plain python3 -m pip install ... to install this utility.

DOWNLOAD_COMMAND ${PYTHON_EXECUTABLE} -m pip install nrfutil

It has now been the responsibility of the user to first create a virtual environment before running cmake. After this, commands like python3 -m pip install ... are fine.

Recommended steps:

Note, all this is only done if developers use cmake options like -DDOWNLOAD_NRFUTIL=ON etc. If developers have their own way to install the (python) tools around bluenet, that's fine as well. There's not even python required if developers only want to cross-compile the bluenet firmware itself and we shouldn't bother such developers.

mrquincle commented 2 years ago

This commit does actually do a nice job in complaining to the user that they are not using a venv when running make, however, it is itself not using it!

When e.g. bluenet_logs is installed through ExternalProject_Add it starts a separate shell to do the pip installation. Also, when we would move this to a cmake script and call it through a -P flag, it can access e.g. the environmental variables in the existing shell, but needs to use execute_process to call pip and will then again use a (sub)shell.

mrquincle commented 2 years ago

The last remark might be true, but it's possible to use execute_process in a separate cmake file. Just make sure that python3 is used and not an absolute path to the binary!

python3 -m pip install bluenet-logs

will work, but for example

/usr/bin/python3.8 -m pip install bluenet-logs

will use global python installation.

mrquincle commented 2 years ago

Would be fixed by https://github.com/crownstone/bluenet/pull/141