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

Deamon stopps working if Sensor not found #39

Closed
JoanMCD opened this issue Apr 24, 2018 · 16 comments
Closed

Deamon stopps working if Sensor not found #39

JoanMCD opened this issue Apr 24, 2018 · 16 comments

Comments

@JoanMCD
Copy link

JoanMCD commented Apr 24, 2018

Hi !
The demon works fine if the miflora sensor is in range. If not the deamon (or the underlaying library) stops working with the message:

Xiaomi Mi Flora Plant Sensor MQTT Client/Daemon
Source: https://github.com/ThomDietrich/miflora-mqtt-daemon

Adding sensor to device list and testing connection ...
Name:          "FloraCare01"
Traceback (most recent call last):
  File "/opt/miflora-mqtt-daemon/miflora-mqtt-daemon.py", line 218, in <module>
    flora_poller.fill_cache()
  File "/usr/local/lib/python3.5/dist-packages/miflora/miflora_poller.py", line 61, in fill_cache
    firmware_version = self.firmware_version()
  File "/usr/local/lib/python3.5/dist-packages/miflora/miflora_poller.py", line 104, in firmware_version
    res = connection.read_handle(_HANDLE_READ_VERSION_BATTERY)  # pylint: disable=no-member
  File "/usr/local/lib/python3.5/dist-packages/btlewrap/gatttool.py", line 23, in _func_wrapper
    return func(*args, **kwargs)
  File "/usr/local/lib/python3.5/dist-packages/btlewrap/gatttool.py", line 253, in read_handle
    raise BluetoothBackendException("Exit read_ble, no data ({})".format(current_thread()))
btlewrap.base.BluetoothBackendException: Exit read_ble, no data (<_MainThread(MainThread, started -1225092352)>)

I test this by simply add a not existing mac in the config.ini
I have installed miflora v0.4. Any ideas ?

@ThomDietrich
Copy link
Owner

Thanks for the bug report!
As I am not able to do serious development right now I would very much appreciate if one of the others reading could have a look. Thanks!

@jboeddeker
Copy link

jboeddeker commented Apr 25, 2018

I enjoy the same. At first, i configured the service as restart=always, but that doesn't help.

I am quite clueless regarding python, but i did a change. I added the "BluetoothBackendException" which is raised by the backend to the "IOError" Exception handler.

Similar is in line 314. Only IOError Exception is handled, but BluetoothBackenException is raised.

But as i said, i am clueless with python, this may be nonsense.

@ThomDietrich
Copy link
Owner

ThomDietrich commented Apr 25, 2018

Hey guys, I had a quick look.
You are on the right track. Simply replace IOError by the new more specific one. As I didn't check if an IOError could still occur, let's catch both.

Please replace those two lines:

by

except (IOError, BluetoothBackendException):

If you can proof the correct behavior please let me know and I'll add the change (or you can push a PR!) Thanks!

@jboeddeker
Copy link

That's exactly what i did. :-D

I am testing it currently. I will do a PR when i am confident this helps.

@JoanMCD
Copy link
Author

JoanMCD commented Apr 25, 2018

@jboeddeker good job, thank you very much!!

line 15:
from btlewrap import available_backends, BluepyBackend, GatttoolBackend, PygattBackend, BluetoothBackendException

@jboeddeker
Copy link

jboeddeker commented Apr 25, 2018

Thanks. I currently just use the import of BluetoothBackendException. Are all of these neccessary?

Line 15:
from miflora.backends.gatttool import GatttoolBackend, BluetoothBackendException

Service doesn't stop anymore, although i have an unreachable sensor.

@JoanMCD
Copy link
Author

JoanMCD commented Apr 25, 2018

hmm, no.
I only want to hint that in comparism with the original code I have to add the BluetoothBackendException

BluepyBackend and PygattBackend are alternative backends. BluepyBackend is more stabile as I read, but I still use GatttoolBackend

@ThomDietrich
Copy link
Owner

Exchanging the backend would be the main task for a next version.
If any of you wants to create a PR with the needed changes, that would be very much appreciated!

@jboeddeker
Copy link

jboeddeker commented Apr 27, 2018

OK, i made the changes. Works fine. But i am confused with github. (Not really used to it, still use SVN for all my projects.)

I made the changes in the version installed on my pi with openHABian. Works.
I created a fork and a branch, but this is a more recent version (some work is done to the backends). Cloned my fork, changed it and this works as well.

But how can i apply the changes to the version which was cloned to my pi by openhabian? This is a simple bugfix and people needing the bugfix, shouldn't go to a new version manually to obtain this.

@ThomDietrich
Copy link
Owner

In order to create a PR here you could actually just work with the GitHub provided editor. As the program is just one script file it is easy to add all changes, then create a PR based on that. After the PR was accepted and merged, you should be able to update your setup on the Pi by re-executing the openhabian-config step.

@ThomDietrich
Copy link
Owner

Hey guys, are you still working on this PR?

@insertjokehere
Copy link
Contributor

I'm running into this (one of my sensors 'drifts' in and out of Bluetooth range, so I'm seeing this pretty often) - will try out the fix @jboeddeker and @JoanMCD over the next few days and put up a PR unless someone beats me to it

LarsAC added a commit to LarsAC/miflora-mqtt-daemon that referenced this issue May 28, 2018
@Hypfer
Copy link

Hypfer commented May 30, 2018

Whats the status on this?

This was referenced May 30, 2018
@kaned
Copy link

kaned commented Jun 10, 2018

Any update on this?

@ThomDietrich
Copy link
Owner

Deeply sorry for the delay guys. This should be solved now.

ThomDietrich pushed a commit that referenced this issue Jun 17, 2018
* Added config dir to simplify docker handling

* Added a Dockerfile

* Implement suggested fix for issue #39

* Improved documentation

* Added documentation on homeasssistant-mqtt reporting mode

* Use local directory as default for config

* Update README.md
@kaned
Copy link

kaned commented Jun 27, 2018

Thomas, thanks for adding this in and maintaining this daemon. It works great and makes it much easier to work with the MiFlora!

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

No branches or pull requests

6 participants