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

Viewer starts without Documents directory #6567

Merged
merged 1 commit into from
Jul 30, 2020

Conversation

aseelegbaria
Copy link
Contributor

@aseelegbaria aseelegbaria commented Jun 10, 2020

Addresses #5707
Tracked on: DSO-13589

Copy link
Collaborator

@ev-mp ev-mp left a comment

Choose a reason for hiding this comment

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

Hi, please verify this behavior under

  1. User path requires admin credentials.
    2 The path points to network resource

common/os.cpp Outdated Show resolved Hide resolved
common/os.cpp Outdated Show resolved Hide resolved
@aseelegbaria aseelegbaria changed the title Documents folder is created if it doesn't exist Viewer starts without Documents directory Jun 15, 2020
Copy link
Collaborator

@maloel maloel left a comment

Choose a reason for hiding this comment

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

As discussed with Aseel, a solution that automatically creates a Documents folder for the user is problematic and not needed.

What is needed is for the Viewer to not crash or behave badly.

Since the folder is controlled by settings, if it does not exist a log should simply not be written.
Same for recording: the folder is controlled by settings. If the folder does not exist, when the user presses Record then we should query the user for a location through the regular means (which should already exist).

common/os.cpp Outdated
else if ( f == user_documents)
{
std::string msg = "Failed to get Documents directory";
rs2::log(RS2_LOG_SEVERITY_INFO, msg.c_str());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not Error?

common/model-views.cpp Outdated Show resolved Hide resolved
@maloel maloel requested a review from ev-mp July 27, 2020 06:06
common/os.cpp Outdated
@@ -287,6 +287,11 @@ Some auxillary functionalities might be affected. Please report this message if
res = str;
res += "\\";
}
else if ( f == user_documents)
{
std::string msg = "Failed to get Documents directory";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pls parse and print the value of HRESULT hr = SHGetKnownFolderPath(... - there are additional failure cases as well

Copy link
Collaborator

@ev-mp ev-mp left a comment

Choose a reason for hiding this comment

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

Looks good, thanks.

@ev-mp ev-mp merged commit 8b088c1 into IntelRealSense:development Jul 30, 2020
@aseelegbaria aseelegbaria deleted the Documents branch May 2, 2021 13:25
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.

3 participants