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

Downgrade RSSDK2 warning message to status #3683

Merged

Conversation

shrijitsingh99
Copy link
Contributor

Resolves #3490
I just suppressed the warning and display a message for now.
Anything else expected or any changes required?

@kunaltyagi kunaltyagi added this to the pcl-1.10.1 milestone Feb 27, 2020
@kunaltyagi
Copy link
Member

The linked issue contains CMake log such as:

-- Could NOT find RSSDK (missing: RSSDK_LIBRARIES RSSDK_INCLUDE_DIRS) 
-- RealSense SDK support: not building because RSSDK not found

It'd be better if the RSSDK2 log were similar to RSSDK.

@kunaltyagi kunaltyagi added module: cmake needs: code review Specify why not closed/merged yet labels Feb 27, 2020
@SergioRAgostinho
Copy link
Member

Preferably yes, but I also recon that in RSSDK we search for the files and directories ourselves, while in RSSDK2 we're using and looking for their config file (correct me if I'm wrong). Either it finds everything or it doesn't. So it might not be worth to aim for this level of uniformed report because the find mechanisms are not exactly the same.

@kunaltyagi
Copy link
Member

But the messaging mechanism is written by us. Just the message(STATUS...) bit needs changing. Output is already limited using the QUIET flag

@SergioRAgostinho
Copy link
Member

Let me clarify then. Having this is ok

-- Could NOT find RSSDK2 

But I wouldn't care about having this

-- Could NOT find RSSDK2 (missing: whatever reasons.) 

@shrijitsingh99
Copy link
Contributor Author

But the messaging mechanism is written by us. Just the message(STATUS...) bit needs changing. The output is already limited using the QUIET flag

As I understood, I updated the status message to be in line with other CMake logs

I didn't use "find_package_handle_standard_args(RSSDK2 DEFAULT_MSG RSSDK2_LIBRARIES RSSDK2_INCLUDE_DIRS)", since as already mentioned it would be redundant to display what's missing in this case.

@kunaltyagi kunaltyagi added needs: pr merge Specify why not closed/merged yet and removed needs: code review Specify why not closed/merged yet labels Feb 28, 2020
@SergioRAgostinho SergioRAgostinho merged commit 05ce484 into PointCloudLibrary:master Feb 28, 2020
@shrijitsingh99 shrijitsingh99 deleted the rssdk2-warning branch March 12, 2020 20:52
@kunaltyagi kunaltyagi removed the needs: pr merge Specify why not closed/merged yet label Mar 15, 2020
FSund pushed a commit to FSund/pcl that referenced this pull request Mar 16, 2020
* Downgrade package not found warning to status

* Update status message
@taketwo taketwo added the changelog: fix Meta-information for changelog generation label Mar 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: fix Meta-information for changelog generation module: cmake
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Downgrade RSSDK2 warning message to status.
4 participants