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

[C++] set_devices_changed_callback fire behavior different between backends after device.hardware_reset() #6921

Open
MojamojaK opened this issue Jul 25, 2020 · 13 comments

Comments

@MojamojaK
Copy link
Contributor


Required Info
Camera Model D415
Firmware Version *
Operating System & Version Win10
Platform PC
SDK Version 2.*.*
Language C++

Issue Description

I was comparing behavior between FORCE_RSUSB_BACKEND=true/false

It seems like the callback set at context.set_devices_changed_callback(T callback) will only be called after a device.hardware_reset() when FORCE_RSUSB_BACKEND=false.

With FORCE_RSUSB_BACKEND=true, the callback won't be fired.

I think the behavior should at least be consistent between the two backends.

@MartyG-RealSense
Copy link
Collaborator

Hi @MojamojaK The link below describes the advantages and disadvantages of the backend installation method compared to the traditional method of compiling from source. It may provide useful insights about the differences of RSUSB true vs false that you have encountered.

#5212 (comment)

Please scroll down through the linked-to comment to the section headed What are the advantages and disadvantages of using libuvc vs patched kernel modules?

@MojamojaK
Copy link
Contributor Author

MojamojaK commented Jul 26, 2020

I understand that there are differences between the two backends. I mean, there are reasons to having both of them.
I am just talking about the behavior of context.set_devices_changed_callback

context.set_devices_changed_callback is a method which is supposed to make a callback fire every time there is a "change in device connectivity", as the name implies.

Currently with FORCE_RSUSB_BACKEND=true, the callback won't fire on device.hardware_reset() which IS a "change in device connectivity".

I personally think this is a bug, but is kind of open to interpretation since there is no official documentation for the set_devices_changed_callback method.

@MartyG-RealSense
Copy link
Collaborator

This particular topic is outside of my programming knowledge unfortunately. One of the other RealSense team members such as @dorodnic may be able to advise on the question above.

@dorodnic
Copy link
Contributor

dorodnic commented Aug 4, 2020

Hi @MojamojaK

Base implementation of device callbacks relies on repeated polling on connected devices. This method is good because its portable across OS-es, but has the drawback of sometimes missing events if camera reconnected before next polling cycle occurs.

On Windows, specifically Win 10, we have implementation that takes advantage of WinAPI functions to get these events faster. RSUSB doesn't do that since it tries to be cross-platform, except for the low level USB library (WinUSB/libusb/USBHost)

My opinion is that it falls within expectations from the API. If Win10 backend is available to you, it is recommended to use it, RSUSB has other issues in addition to enumeration events. Of course, this project is open-source, and you are welcomed to customise it to your needs. To get this functionality on with RSUSB on Windows, you'd need to take win_event_device_watcher from mf-backend and put it here - /src/win7/rsusb-backend-windows.cpp:L19

@MojamojaK
Copy link
Contributor Author

Thanks very much for the guidance.
It might take time since I'm working on something else now but I'll try to implement it.

@dorodnic
Copy link
Contributor

dorodnic commented Aug 4, 2020

Great
Another, even quicker alternative, may be decreasing the timeout here -

if(cancellable_timer.try_sleep(5000))

Right now RSUSB is checking device changes every 5 seconds.
Reducing this constant will reduce chance of missing events, but it will increase (slightly) idle CPU utilisation

@MojamojaK
Copy link
Contributor Author

Yes, I kind of noticed that.
Do you think it would be better/reasonable to make the polling interval more configurable through CMake or something?

@dorodnic
Copy link
Contributor

dorodnic commented Aug 5, 2020

It seems like a good idea. This timeout used to be lower, but these queries do end-up, in terms of CPU utilisation. Please consider creating a pull-request.
Ultimately, I'd like the development team to write custom device watches for different RSUSB flavours.

@MartyG-RealSense
Copy link
Collaborator

Just leaving a note that this case should be kept open whilst the pull request is still active. Thanks for contributing the pull @MojamojaK :)

@MartyG-RealSense
Copy link
Collaborator

Note to @dorodnic that the PR of @MojamojaK has passed checks and is awaiting an approving review in order for merging to proceed. Thanks!

#7085

@MartyG-RealSense
Copy link
Collaborator

Adding a reminder for @dorodnic that the pull of @MojamojaK has passed checks and is awaiting an approving review in order for merging to proceed. Thanks!

#7085

@MartyG-RealSense
Copy link
Collaborator

@dorodnic - please see comment above. Thank you.

@MartyG-RealSense
Copy link
Collaborator

Reminder for @dorodnic that the pull of @MojamojaK has passed checks and is awaiting an approving review in order for merging to proceed. Thanks!

#7085

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

No branches or pull requests

3 participants