Skip to content
This repository has been archived by the owner on Aug 27, 2022. It is now read-only.

Feature/error log #277

Closed
wants to merge 6 commits into from
Closed

Feature/error log #277

wants to merge 6 commits into from

Conversation

Metropo
Copy link

@Metropo Metropo commented Jul 29, 2021

Prerequisites checklist

What is the purpose of this pull request? (put an "x" next to an item)

  • New feature

What changes did you make? (Give an overview)

After building my own version of the photobooth and using it for some occasions i hat the issue that multible times no images where taken, because gphoto returned an error. Because the error is only briefly shown in the debug console of the browser triggering the foto and not really containing the real error, because it was written to stderr and not redirected, it was very hard to troubleshoot the issue.
Because of this i added stderr to stdout redirect and a mechanism to write the reasons for failed photos in a file that can be viewed from the admin panel.

Example:

2021-07-29T12:18:28+02:00:
Array
(
[error] => File was not created
[cmd] => gphoto2 --capture-image-and-download --filename=/var/www/html/data/tmp/20210729_121826.jpg 2>&1
[returnValue] => 0
[output] => Array
(
[0] =>
[1] => *** Error ***
[2] => Out of Focus
[3] => ERROR: Could not capture image.
[4] => ERROR: Could not capture.
)
)

Is there anything you'd like reviewers to focus on?

Changes are extensivly tested under Linux but not using windows, but it should work in theroy ;)

@andi34
Copy link
Owner

andi34 commented Jul 29, 2021

Hey and thanks for your contribution!
@jacques42 started working on a debug panel on #275 , maybe this can be combinined?

@Metropo
Copy link
Author

Metropo commented Jul 29, 2021

Would probably an option.
The only issues I see at the moment are:
It seems that the debug panel is only visible in Dev mode, which disables gphoto and uses sample images i guess?
And in my opinion the log can also help less expierenced users to find errors like not switching on the camera or using the wrong focus mode.
But yes, in general the debug panel seems to be a nice destination for this log.

@jacques42
Copy link
Collaborator

Hey @Metropo many thanks for your contributions, I like it a lot.

The debug panel will be accessible at any time, regardless of dev mode on or off. My comment in the PR is confusing, I wanted to indicate that dev mode gives more information in cases. I will update that comment.

Definitely I'd propose to merge - making this log accessible as a separate entity would break the concept of having a debug panel.

The debugpanel is ready to merge to dev. Would you prefer me to do this first then rebase or any other approach ?

@Metropo
Copy link
Author

Metropo commented Jul 29, 2021

Hi, @jacques42 I also like your idea a lot and i think it is really useful :) Of cause it is the only clean solution to add the picture log in there.
I will revert my changes in the ui, so that we don't have a seperate log button. If you want you can add or prepare the new tab, otherwise i can do this as well and we merch everything in dev :)

@andi34 andi34 mentioned this pull request Jul 29, 2021
2 tasks
@Metropo
Copy link
Author

Metropo commented Jul 30, 2021

@jacques42 I have reverted all UI-related changes now. So only the logfile is created 👍

@jacques42
Copy link
Collaborator

@Metropo I merged the debugpanel to dev, and it's already prepared to show the camera gphoto log file, once it exists. Unfortunately now this seems has caused a merge conflict. I'm happy to look into it but need a bit leadtime, or you can if you are up for it. Again - many thanks for your contributions, this is really valuable and we are super happy to have you with us.

@andi34
Copy link
Owner

andi34 commented Jul 30, 2021

I can look at it later in the evening

@Metropo
Copy link
Author

Metropo commented Jul 30, 2021

I just came home and wanted to have a look at it, but i dont see any conflicts. i could merge dev into my branch without problems.
Edit: Maybe the conflict was from the verstion prior to reverting all ui changes?

@andi34
Copy link
Owner

andi34 commented Jul 30, 2021

Merge and squashed merge would have worked but I prefer rebate & merge which isn't possible. But don't worry, I'll push your changes in some minutes to the dev branch

@Metropo
Copy link
Author

Metropo commented Jul 30, 2021

@andi34 ah that explaines it. I can also rebase everythin cleanly on my branch if you prerfer :) was not aware of this. Just let me know

@andi34 andi34 closed this in 48b7c4a Jul 30, 2021
@andi34
Copy link
Owner

andi34 commented Jul 30, 2021

Added to dev branch :)

@Metropo
Copy link
Author

Metropo commented Jul 30, 2021

thank you very much :)

@Metropo Metropo deleted the feature/error_log branch July 30, 2021 20:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants