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.

Sign up for GitHub

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

Jump to bottom

Fixed pose_lookup and dynamic_lib #304

Merged
merged 7 commits into from
Feb 21, 2022
Merged

Fixed pose_lookup and dynamic_lib #304

merged 7 commits into from
Feb 21, 2022

Conversation

e3m3
Copy link
Contributor

@e3m3 e3m3 commented Jun 3, 2021 •
edited

Closes #303 .

  • Fixed pose_lookup initialization in pose_lookup/plugin.cpp
  • Added debugging prints to common/dynamic_lib.hpp
  • New path_basename for debugging in common/dynamic_lib.hpp
  • Failed calls to dlclose non-fatal with dbg in common/dynamic_lib.hpp

@e3m3 e3m3 added bug Something isn't working plumbing Something that connects/interfaces/plugs into many parts of ILLIXR. labels Jun 3, 2021
@e3m3 e3m3 added this to the 2.x milestone Jun 3, 2021
@e3m3 e3m3 self-assigned this Jun 3, 2021
@e3m3 e3m3 added this to PRs in Progress in ILLIXR improvements via automation Jun 3, 2021
charmoniumQ
charmoniumQ reviewed Jun 3, 2021
View reviewed changes
Copy link
Member

@charmoniumQ charmoniumQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imo, the -DNDEBUG and normal builds have deviated too far in dynamic_lib. One has a destructor, the other doesn't.

Can we merge the paths and always propagate debug information?

@e3m3
Copy link
Contributor Author

e3m3 commented Jun 3, 2021

Partially addresses #305 for dbg. Does not resolve root issue with missing symbol reference to VTK in debugview.

@e3m3 e3m3 mentioned this pull request Jun 3, 2021
Closed
2 tasks
@e3m3 e3m3 requested review from charmoniumQ and mhuzai June 3, 2021 14:55
charmoniumQ
charmoniumQ reviewed Jun 3, 2021
View reviewed changes
Copy link
Member

@charmoniumQ charmoniumQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I intend dynamic_lib to be used outside of ILLIXR, and in that case, the basename of the dirname is not so meaningful. Let's set the name to the while pathname.

Then lgtm.

@e3m3 e3m3 requested a review from charmoniumQ June 3, 2021 22:08
charmoniumQ
charmoniumQ reviewed Jun 23, 2021
View reviewed changes
common/dynamic_lib.hpp Outdated Show resolved Hide resolved
@e3m3 e3m3 force-pushed the issue-303-fix-pose-lookup branch from b9ee19b to 3ccef55 Compare July 9, 2021 19:56
charmoniumQ
charmoniumQ reviewed Jul 22, 2021
View reviewed changes
common/dynamic_lib.hpp Outdated Show resolved Hide resolved
@e3m3 e3m3 force-pushed the issue-303-fix-pose-lookup branch from 3ccef55 to 9b51e19 Compare October 5, 2021 19:49
@e3m3 @mhuzai
Fixing pose_lookup and dynamic_lib (#303)
44c0c0a 
- Fixed 'pose_lookup' initialization in 'pose_lookup/plugin.cpp'
- Added debugging prints to 'common/dynamic_lib.hpp'
- New 'path_basename' for debugging in 'common/dynamic_lib.hpp'
- Failed calls to 'dlclose' non-fatal with 'dbg' in 'common/dynamic_lib.hpp'
@e3m3 @mhuzai
Updated literal types in 'commmon/dynamic_lib.hpp' (#303)
54a1384
@e3m3 @mhuzai
Addressing reviewer comments (#303)
7f2c48e 
- Merged 'opt' and 'dbg' paths in 'common/dynamic_lib.hpp'
@e3m3 @mhuzai
Miscellaneous changes (#303)
3bfa254 
- Committing to mitigate CI initialization bug (gtsam preintegration)
- Refactored spacing in 'common/dynamic_lib.hpp'
@e3m3 @mhuzai
Updating 'dynamic_lib' (#303)
bd62c97 
- Changed '_m_lib_name' -> '_m_lib_path' in 'common/dynamic_lib.hpp'
- Added comments in 'common/dynamic_lib.hpp'
@e3m3 @mhuzai
Using string constructor copy in 'common/dynamic_lib.hpp' (#303)
94c4131
@e3m3 @mhuzai
Removed debug guard when failing to open a dynamic library (#303
5e4c8fb
ILLIXR improvements automation moved this from PRs in Progress to PRs Approved Feb 21, 2022
@charmoniumQ charmoniumQ merged commit 4f582cb into master Feb 21, 2022
ILLIXR improvements automation moved this from PRs Approved to Done Feb 21, 2022
@charmoniumQ charmoniumQ mentioned this pull request Feb 21, 2022
Closed
@mhuzai mhuzai deleted the issue-303-fix-pose-lookup branch April 8, 2022 06:36
JeffreyZh4ng added a commit that referenced this pull request May 3, 2022
@JeffreyZh4ng
Semi working with reader using sequential reads
1a562a2 
Semi working offloading. VIO doesnt work ATM

test commit

Add offloading server and device code

Fixing common and adding extra prints for debugging

Add prints and offloading config

Server fix for zed pt1

Offloading device cleanup

Add offload client config

Revert boost install script (#323)

* Revert "Enabled boost in 'deps.sh' (#308) (#309)"

This reverts commit 8590c74.
We do not need to build boost ourselves, we can install it from apt.

* Do not prompt user for boost installation

Fix docs (#319)

* Fix writing_your_plugin.md docs

* Added Bill's suggestions

* Fix typos

* Fix typos

Fixed `pose_lookup` and `dynamic_lib` (#304)

* Fixing pose_lookup and dynamic_lib (#303)

- Fixed 'pose_lookup' initialization in 'pose_lookup/plugin.cpp'
- Added debugging prints to 'common/dynamic_lib.hpp'
- New 'path_basename' for debugging in 'common/dynamic_lib.hpp'
- Failed calls to 'dlclose' non-fatal with 'dbg' in 'common/dynamic_lib.hpp'

* Updated literal types in 'commmon/dynamic_lib.hpp' (#303)

* Addressing reviewer comments (#303)

- Merged 'opt' and 'dbg' paths in 'common/dynamic_lib.hpp'

* Miscellaneous changes (#303)

- Committing to mitigate CI initialization bug (gtsam preintegration)
- Refactored spacing in 'common/dynamic_lib.hpp'

* Updating 'dynamic_lib' (#303)

- Changed '_m_lib_name' -> '_m_lib_path' in 'common/dynamic_lib.hpp'
- Added comments in 'common/dynamic_lib.hpp'

* Using string constructor copy in 'common/dynamic_lib.hpp' (#303)

* Removed debug guard when failing to open a dynamic library (#303

Added documentation for using Switchboard and Phonebook externally (#320)

* Added documentation for using Switchboard and Phonebook externally

* fix typo and spacing

* Fix typos

Co-authored-by: Qinjun Jiang <90299267+qinjunj@users.noreply.github.com>

Renamed files and merge master

Cleaned up code ready for PR

Fix readme

Revert hotfix and address comments

Revert rt_slam config

Minor fix and verify all combinations of ZED/dataset with OV/Kimera work
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@charmoniumQ charmoniumQ charmoniumQ approved these changes

@mhuzai mhuzai mhuzai approved these changes

@jianxiapyh jianxiapyh Awaiting requested review from jianxiapyh

Assignees

@e3m3 e3m3

Labels
bug Something isn't working plumbing Something that connects/interfaces/plugs into many parts of ILLIXR.
Projects
ILLIXR improvements
  
Done
Milestone
3.x
Development

Successfully merging this pull request may close these issues.

pose_lookup initialization is buggy
3 participants
@e3m3 @charmoniumQ @mhuzai