IntelRealSense / librealsense

Intel® RealSense™ SDK
https://www.intelrealsense.com/
Apache License 2.0
7.6k stars 4.83k forks source link

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

Open MojamojaK opened 4 years ago

MojamojaK commented 4 years ago

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 commented 4 years ago

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.

https://github.com/IntelRealSense/librealsense/issues/5212#issuecomment-552184604

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

Great Another, even quicker alternative, may be decreasing the timeout here - https://github.com/IntelRealSense/librealsense/blob/f7cdf6e8961e1709f6d864bdb33095c00a671ca7/src/types.h#L1544 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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

https://github.com/IntelRealSense/librealsense/pull/7085

MartyG-RealSense commented 4 years ago

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!

https://github.com/IntelRealSense/librealsense/pull/7085

MartyG-RealSense commented 4 years ago

@dorodnic - please see comment above. Thank you.

MartyG-RealSense commented 3 years ago

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

https://github.com/IntelRealSense/librealsense/pull/7085