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

Re-setting notifications for sensor fusion module #53

Open
bgromov opened this issue Aug 14, 2018 · 3 comments
Open

Re-setting notifications for sensor fusion module #53

bgromov opened this issue Aug 14, 2018 · 3 comments

Comments

@bgromov
Copy link
Contributor

bgromov commented Aug 14, 2018

  • pymetawear version: v0.11.0
  • Python version: 2.7.10
  • Operating System: Linux / macOS

Description

I would like to selectively enable/disable notifications for the Sensor Fusion module, but with the current API / implementation it seems impossible.

What I Did

# Enable the following callbacks
self.mwc.sensorfusion.notifications(
    quaternion_callback = self.mwc_quat_cb,
    corrected_acc_callback = self.mwc_acc_cb,
    corrected_gyro_callback = self.mwc_gyro_cb)

# Disable just one
self.mwc.sensorfusion.notifications(
    quaternion_callback = self.mwc_quat_cb,
    corrected_acc_callback = None,
    corrected_gyro_callback = self.mwc_gyro_cb)

The second call to sensorfusion.notifications() results in exception:

Subscription to {0} signal already in place!

Obviously, if I omit the other two callback parameters the respective callbacks would be also disabled.

Is there a way to selectively enable/disable these notifications?

Thanks!

@bgromov
Copy link
Contributor Author

bgromov commented Aug 15, 2018

Looks like the only problem is in the exception itself, i.e. commenting these lines out doesn't do any visible harm:

if self.callback is not None:
raise PyMetaWearException(
"Subscription to {0} signal already in place!")

@hbldh
Copy link
Owner

hbldh commented Aug 15, 2018

I wanted to keep the notifications method as similar to the other modules as possible, but it breaks possibility to handle the different signals separately. I will have to restructure this I guess.

I am a bit perplexed by the notifications method of the sensor fusion module going back to the place you indicate. It should never get there. I will examine this closer.

There was an abyssmal error in the error message as well: "Subscription to {0} signal already in place!" without a .format(self.module_name) or similar...

@bgromov
Copy link
Contributor Author

bgromov commented Aug 15, 2018

I just checked again: actually the error has nothing to do with the Sensor Fusion... Sorry for the false alarm! In fact the error comes from enabling battery notifications twice (which is also a bit of an issue).

While working on the PR #54 I noticed check_and_change_callback() in sensor fusion module that seems to do exactly what I want, i.e. replaces callbacks and enables/disables individual data signals. The only problem is that even if the callback is exactly same, the data signal will be switched off and on again, so potentially some data can be missed.

Perhaps it would be good to use the logic of check_and_change_callback() in the other modules as well, including the base.

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

2 participants