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

Feat gui telegram #24

Merged
merged 30 commits into from
Sep 16, 2024
Merged

Feat gui telegram #24

merged 30 commits into from
Sep 16, 2024

Conversation

PhilippMundhenk
Copy link
Owner

@PhilippMundhenk PhilippMundhenk commented Sep 15, 2024

@vgarcia007. This is the PR to merge your changes into master. I also added you as collaborator, so you should be able to edit.

Good news: Initial tests show that everything works as intended, this is great! And the GUI is simply amazing! The status updates are awesome, the automatic loading of scans, the day of the scan in the list, etc. I love it! If you couldn't tell form the previous one: Frontends and Design are not my strong side.

Three small items I noticed:

  • When pressing the button in the frontend, it remains highlighted, even after the scanning process is over. Might make sense to reset the highlight on status update. Probably a small thing, but would likely take me a day to fix...
  • It seems status always shows "Waiting for Rear Pages", even if rear pages have been scanned and thus waiting does no longer occur. Could it be that this is a flaw in my scripts and the suprocess in the front page scanning only exits after being killed and usleep finishes? This would explain. Maybe checking for existence of /tmp/${date}/scan_pid could be more stable?
  • It seems the weekday in the download list is in my case always "Sun". Is this related to when the server is started? Modification time in Windows shows correctly...

@vgarcia007
Copy link
Collaborator

You're right—the button highlight should disappear. I'll look into it.

The "Waiting for Rear Pages" issue is interesting. It worked on my machine 😅. To be honest, I just grepped the process I thought was correct without fully understanding your shell script. I'll run more tests and check your suggestion regarding "/tmp/${date}/scan_pid".

The issue with the weekday might be related to timezones. I know from other projects that they can be tricky. Perhaps setting the timezone in the compose file and in PHP could resolve it.

I'm not sure how quickly I can address everything, but I promise to follow up soon.

Button Issue Resolved, but "Waiting for Rear Pages" Remains a Challenge

I believe the issue lies within the shell (.sh) files.

When the user presses "Scan Front Pages", the status updates to "Scanning in Progress" (this is done by checking if the scanimage process is running).

After the front page scan completes, the status changes to "Waiting for Rear Pages" (this is determined by checking if the sleep process is running). The script waits for 2 minutes to give the user time to insert the rear pages into the scanner and press "Scan Rear Pages".

If the user does not press "Scan Rear Pages" within the 2-minute window, the script assumes no rear pages are present and proceeds directly to OCR. The status then changes to "OCR in Progress" (this checks if the curl process is running).

If the user presses "Scan Rear Pages" within the 2-minute window, the rear page scan is initiated, and the status returns to "Scanning in Progress" (again checking if the scanimage process is running).

Issue:
The rear page scan is no longer triggered. As a result, the countdown continues, and pressing the "Scan Rear Pages" button has no effect. The issue was initially due to missing execution permissions on scantoemail-0.2.4-1.sh, which has now been fixed.

Post-Fix Behavior:
5. After pressing "Scan Rear Pages", the status correctly changes to "Scanning in Progress" (indicating that the scanimage process is running).

The script then sends the files to the OCR server, with the status updating to "OCR in Progress" (this checks if the curl process is running).
Now, here’s where things get complicated. It seems that the 120-second countdown is still running in the background. If a user scans both the front and rear pages within this 120-second window and OCR finishes, the status reverts back to "Waiting for Rear Pages."

This is happening because:

The scanimage process is not running.
The curl process is not running.
But the sleep process is still active.
To be honest, I'm not entirely sure what's happening in all of the .sh files. Is it possible for the scanRear.sh script to kill all sleep processes and also terminate the main scantofile.sh at the end, or could that cause issues elsewhere?

Regarding the Issue with the Weekdays

I suspect this might be a timezone-related issue. I have removed the UTC timezone setting from the Dockerfile, so it should now use the timezone (TZ) defined in the compose file. I also added a timezone.php script to verify which timezone PHP is using within the container. Hopefully, this will help with further investigation. Unfortunately, I’m unable to fully resolve the issue at the moment.

Let me know if you'd like further adjustments or clarifications.
@vgarcia007
Copy link
Collaborator

Button Issue Resolved, but "Waiting for Rear Pages" Remains a Challenge

I believe the issue lies within the shell (.sh) files.

When the user presses "Scan Front Pages", the status updates to "Scanning in Progress" (this is done by checking if the scanimage process is running).

After the front page scan completes, the status changes to "Waiting for Rear Pages" (this is determined by checking if the sleep process is running). The script waits for 2 minutes to give the user time to insert the rear pages into the scanner and press "Scan Rear Pages".

If the user does not press "Scan Rear Pages" within the 2-minute window, the script assumes no rear pages are present and proceeds directly to OCR. The status then changes to "OCR in Progress" (this checks if the curl process is running).

If the user presses "Scan Rear Pages" within the 2-minute window, the rear page scan is initiated, and the status returns to "Scanning in Progress" (again checking if the scanimage process is running).

Issue:
The rear page scan is no longer triggered. As a result, the countdown continues, and pressing the "Scan Rear Pages" button has no effect. The issue was initially due to missing execution permissions on scantoemail-0.2.4-1.sh, which has now been fixed.

Post-Fix Behavior:
5. After pressing "Scan Rear Pages", the status correctly changes to "Scanning in Progress" (indicating that the scanimage process is running).

The script then sends the files to the OCR server, with the status updating to "OCR in Progress" (this checks if the curl process is running).
Now, here’s where things get complicated. It seems that the 120-second countdown is still running in the background. If a user scans both the front and rear pages within this 120-second window and OCR finishes, the status reverts back to "Waiting for Rear Pages."

This is happening because:

The scanimage process is not running.
The curl process is not running.
But the sleep process is still active.
To be honest, I'm not entirely sure what's happening in all of the .sh files. Is it possible for the scanRear.sh script to kill all sleep processes and also terminate the main scantofile.sh at the end, or could that cause issues elsewhere?

Regarding the Issue with the Weekdays

I suspect this might be a timezone-related issue. I have removed the UTC timezone setting from the Dockerfile, so it should now use the timezone (TZ) defined in the compose file. I also added a timezone.php script to verify which timezone PHP is using within the container. Hopefully, this will help with further investigation. Unfortunately, I’m unable to fully resolve the issue at the moment.

Let me know if you'd like further adjustments or clarifications.

changed process killing to also kill subprocesses of waiting OCR trigger
@PhilippMundhenk
Copy link
Owner Author

Your analysis is spot on! The kill interrupts the process (i.e., the running shell script), but not the sleep call, which first needs to finish executing, before returning back to the killed shell script, thus terminating. replacing kill -9 with pkill -P solves this and kills the script and all subprocesses (such as sleep).

Also the timezon issue seems to be correct. My Docker is set to Europe/Berlin, but /timezone.php shows Timezone: UTC. How can we adapt this? It is a small issue and shouldn't block us, but would be neat, if it works...

Fixed weekday in list
@PhilippMundhenk
Copy link
Owner Author

PhilippMundhenk commented Sep 16, 2024

Ok, I think it is not the timezone, but it is rather $mtime being empty. Shouldn't it be $attributes['mtime']? See previous commit.

Since I created this PR, I apparently can't approve it, even if in my own repo. I could only disable PR reviews altogether. If these changes work for you, feel free to officially review this PR, @vgarcia007. Then we can merge. Everything else lgtm.

minor changes for consistency
Minor changes for consistency
@vgarcia007
Copy link
Collaborator

You are right. It has to be $attributes['mtime']. Nevertheless, the time zone is still not correct. But I think that’s a smaller problem. I also have some ideas on how we can fix that as well. But we can create a separate issue for that later.

Copy link
Collaborator

@vgarcia007 vgarcia007 left a comment

Choose a reason for hiding this comment

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

Looks OK and seems to work!

@vgarcia007 vgarcia007 merged commit ea5467b into master Sep 16, 2024
1 check passed
@PhilippMundhenk
Copy link
Owner Author

Love it! Thank you so much for this contribution! Since yesterday, I find myself using the GUI more than the buttons. It is just more fun like this! ;)

@vgarcia007
Copy link
Collaborator

I'm so glad to hear that you're enjoying it! It’s awesome to know that the GUI is making things more fun. Your feedback really motivates me – thank you!

@PhilippMundhenk PhilippMundhenk deleted the feat_GuiTelegram branch September 22, 2024 18:13
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