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

Fix channel activation before Connect #195

Open
f00f opened this issue Apr 5, 2019 · 4 comments
Open

Fix channel activation before Connect #195

f00f opened this issue Apr 5, 2019 · 4 comments
Labels

Comments

@f00f
Copy link
Contributor

f00f commented Apr 5, 2019

When channels are activated before Connect they will be added to ActiveChannels and ActivateChannelImpl will be called for them by Connect.

For the Orbbec cameras this will cause a malfunction of both ZImage and Point3DImage are activated before Connect. As a result none of these channels will actually be activated.

Possible actions to solve this:

  1. Drop support for the Point3DImage channel in the Orbbec.
    The channel is broken anyways and only computed by the driver.
    This would fix the problem for the Orbbec camera only.
  2. Replace the IsChannelActive checks in ActivateChannel by DepthStream.isValid().
    This would fix the problem for the Orbbec camera only.
  3. Modify the ActivateChannelImpl behaviour:
    Return true if the channel was activated and no further action is needed.
    Return false if channel activation was not possible, and ActivateChannelImpl should be called again for this channel after ConnectImpl.
    Based on the return value, the channel would be added to ActiveChannels or a new list, e.g. ChannelsToActivateAfterConnect (naming suggestions welcome).
    I would then also rename ActivateChannelImpl to TryActivateChannelImpl.
    The behaviour of DeactivateChannel must be changed accordingly (e.g. activating and deactivating a channel before Connect should result in that channel not being in any list.
    This would be a larger change to the internal interface and solve it for all cameras.

Originally posted by @sisiplac in #194

@f00f f00f added the bug label Apr 5, 2019
@f00f
Copy link
Contributor Author

f00f commented Apr 5, 2019

@sisiplac feel free to make any changes to the description

@sisiplac
Copy link
Contributor

sisiplac commented Sep 3, 2019

The bug is fixed using option 2 in 9559aed
@f00f:
We could transfer option 3 in an own enhancement ticket or just delete it completely - what do you think?

@f00f
Copy link
Contributor Author

f00f commented Sep 5, 2019

I think I still like option 3, so I would keep it around as an issue. Whenever we run into a similar problem we can dig it out.

@sisiplac
Copy link
Contributor

sisiplac commented Sep 6, 2019

Yes - that is very likely a good decision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants