-
Notifications
You must be signed in to change notification settings - Fork 10
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
Initialize the rotation estimator pointer in realsense2withIMUDriver constructor #36
Initialize the rotation estimator pointer in realsense2withIMUDriver constructor #36
Conversation
…realsense2withIMUDriver constructor
@GiulioRomualdi do you expect more fixes? Because it seems that we have several changes that would need a release, but if you have more fixes that you expect to do we can wait. |
I just realize that with the documentation I wrote, I cannot get the output of the camera but just a port named I don't know how can I run two devices ( |
Using |
Hi @Nicogene thank you 😄 , I'm trying to write an |
Not sure if it is the same problem but, we experienced low fps passing from Could you try with |
I think the problem is a bit more complex. yarp-device-realsense2/src/devices/realsense2/realsense2withIMUDriver.cpp Lines 369 to 372 in d4da73f
yarp-device-realsense2/src/devices/realsense2/realsense2withIMUDriver.cpp Lines 421 to 424 in d4da73f
The same happens when an image is requested yarp-device-realsense2/src/devices/realsense2/realsense2Driver.cpp Lines 1028 to 1031 in d4da73f
I think this is happening with this configuration because we have two sources of data handled by two different threads ( A possible solution (is to call |
Yes, this is tipically how YARP devices are implemented. Either you have a thread that blocks to wait for data, or you use callbacks to populate some internal buffers. Not sure if this is problematic for example for the image, as it means that we have some additional copy of the image. |
Looking at the code it seems that data is always copied to the |
It is something that should be tested, but the solution provided by @GiulioRomualdi seems ok 👍🏻 |
What do you think @GiulioRomualdi @Nicogene , should we merge? I do not think there is an high possibility of regression here as this was fixing something that was not working fine. |
I would merge as well 👍🏻 |
As you prefer, but keep in mind that enabling the imu will slow down the rate at which the images arrive. So in case someone needs the IMu I would suggest using the ros2 node |
I agree, but this happens also before this PR, right? |
Before the PR the device segfaults if attached to a multipleanalogsensorserver |
Good point, so basically with this modification the method is not segfaulting anymore, but the performances are really bad, right? So probably it may be better to have a clear error, rather then a behaviour that seems ok but it actually bad. At this point, let's close this PR and open an issue, people interested in this feature may resurrect this PR if interested in this. |
Issue in #37, for the time being we close the PR. |
This fixes #35, moreover, I also wrote the documentation for the RealSense D435i in the README
cc @traversaro