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

Improve error message in check_rs2_error for unknown exception type #16

Closed

Conversation

emilk
Copy link

@emilk emilk commented Jun 12, 2023

I am trying to get realsense working on an M1 MacBook. I installed librealsense 2.54.1 with brew install librealsense. I then tried to run demo_435i and got this:

thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', realsense-rust-1.1.0/src/device.rs:82:13
stack backtrace:
   0: rust_begin_unwind
             at std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at core/src/panicking.rs:64:14
   2: core::panicking::panic
             at core/src/panicking.rs:111:5
   3: core::option::Option<T>::unwrap
             at core/src/option.rs:778:21
   4: realsense_rust::device::Device::try_create
             at realsense-rust-1.1.0/src/device.rs:82:13
   5: realsense_rust::context::Context::query_devices
             at realsense-rust-1.1.0/src/context.rs:118:23

With this PR applied this is improved to:

thread 'main' panicked at 'Unknown Rs2Exception: 8', realsense-rust/src/device.rs:82:13
stack backtrace:
   0: rust_begin_unwind
             at std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at core/src/panicking.rs:64:14
   2: realsense_rust::device::Device::try_create::{{closure}}
             at realsense-rust/src/error.rs:150:25
   3: core::option::Option<T>::unwrap_or_else
             at core/src/option.rs:828:21
   4: realsense_rust::device::Device::try_create
             at realsense-rust/src/device.rs:82:13
   5: realsense_rust::context::Context::query_devices
             at realsense-rust/src/context.rs:118:23

I still don't know why we get an 8 here though, considering there are only eight errors, numbered 0-7: https://intelrealsense.github.io/librealsense/doxygen/rs__types_8h.html#ab73d6772c40d1a2fba645a1d0a7eed5e

Perhaps some version mismatch?


PS: in your README.md and on https://crates.io/crates/realsense-rust you say you are happy to receive PRs on GitHub (which is why opened this PR), but now I see that CONTRIBUTING.md says you only accept them on GitLab 😬

@ThatGeoGuy
Copy link
Member

Hey, thanks for the submission!

Apologies about the GitLab / GitHub confusion. We do take contributions on both sites, but we need to move the branch over to GitLab for the purpose of running CI (since we don't have any GitHub actions setup).

I don't have a ton of time to review this ASAP, but I'm more than happy to duplicate this over to GitLab at some point this week and review there proper. We'll leave this open until then.

@emilk
Copy link
Author

emilk commented Jun 14, 2023

No hurry on my end!

@ThatGeoGuy
Copy link
Member

I've ported it over to GitLab: https://gitlab.com/tangram-vision/oss/realsense-rust/-/merge_requests/50

I will review this shortly -- I can put feedback here and then move work over as necessary.

@ThatGeoGuy
Copy link
Member

Changes seem good, although there seems to be some new lint / test errors that have cropped up due to updates in librealsense.

I need to update the library to 2.54.0 or whatever the latest is, and then probably rebase this off of that work before we merge.

@ThatGeoGuy
Copy link
Member

https://gitlab.com/tangram-vision/oss/realsense-rust/-/merge_requests/50

Alright, this was reviewed & merged. Apologies for the wait. I'm going to close this now if everything is fine here!

@ThatGeoGuy ThatGeoGuy closed this Aug 15, 2023
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

Successfully merging this pull request may close these issues.

2 participants