OpenNI2 icon indicating copy to clipboard operation
OpenNI2 copied to clipboard

Path resolution based on OpenNI2.dll directory (plus xnOSGetDirName() fix)

Open tomoto opened this issue 12 years ago • 10 comments

Hi OpenNI folks,

Here is a patch to address the issue I pointed out in the community at http://community.openni.org/openni/topics/search_path_for_redist_files. With this patch, the path for the drivers and OpenNI.ini will be resolved based on OpenNI2.dll's directory so that the users are no longer required to launch the application from the designated directory. It also allows them to use the shared driver repository just by setting PATH environment variable to its master OpenNI2.dll.

This patch includes three commits as follows. All the details are explained in their commit comments.

  • Commit 1: Fix of an issue of Windows version of xnOSGetDirName(). Even if you did not agree on the path resolution fix, I believe you would like to merge this bug fix.
  • Commit 2: Fix of the path resolution, which is the main part of this contribution. There are some features that I personally believe no longer needed (or possibly even confusing) but left untouched.
  • Commit 3: Patch to remove the features I personally believe no longer needed. You can exclude this commit if you do not agree on my suggestion explained in the commit comment.

I hope this patch to be reviewed (and accommodated if you agree) as early as possible since this is an external behavior change that may impact on the beta users although it is (I believe) a significant improvement.

Please get back to me if you have any questions. Thank you very much, Tomoto

tomoto avatar Jan 07 '13 09:01 tomoto

I may add another commit if I can apply the same method for PS1080.ini. Thanks.

tomoto avatar Jan 07 '13 10:01 tomoto

I have done the work for PS1080.ini and committed. Thanks.

tomoto avatar Jan 07 '13 11:01 tomoto

Benn (piedar) found my code did not compile with gcc (on Linux). We are working on it at https://github.com/tomoto/OpenNI2/pull/1.

tomoto avatar Jan 08 '13 04:01 tomoto

I decided to build my own Linux environment to investigate the problems reported by Benn, and eventually completed the working implementation! My quick testing confirmed the paths were resolved based on the lib*.so's. Please see the commit 86efc7a.

tomoto avatar Jan 08 '13 11:01 tomoto

This path resolution branch is working nicely for me on Gentoo Linux x86_64.

piedar avatar Jan 08 '13 18:01 piedar

Hi Benn,

Thank you very much for your contribution to find the issue and validate the fix! I really appreciate it!

Tomoto

tomoto avatar Jan 08 '13 19:01 tomoto

I merged this pull request into the develop branch after the peer review by Eddie.

tomoto avatar Jan 15 '13 14:01 tomoto

This was merged in, but then immediately overwritten by the following commit. Was this intentional or an accident?

kubat avatar Mar 23 '13 17:03 kubat

When I asked the same thing, I was told it was an accident. But I think this ended up getting stranded in the develop branch. I wonder what the plan is for merging from there to master...

piedar avatar Apr 04 '13 18:04 piedar

I think piedar is right. The patch 4eec88bdb7 was immediately overwritten by b8aae20cf4 by mistake, and then recovered by 743fbda191 but it is still only in the development branch. I hope it will be merged into the master in the merge.

tomoto avatar Apr 05 '13 17:04 tomoto