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

Fixfeat temp dir device name #38

Merged
merged 6 commits into from
Sep 22, 2024
Merged

Conversation

PhilippMundhenk
Copy link
Owner

@pedropombeiro: This is the PR to get this to master, after manual testing.

@PhilippMundhenk PhilippMundhenk marked this pull request as draft September 22, 2024 18:01
@pedropombeiro pedropombeiro marked this pull request as ready for review September 22, 2024 18:02
@PhilippMundhenk
Copy link
Owner Author

Interestingly, in this PR, I get the issues fixed in #36, as well. Didn't see them earlier...

too lazy to rebase
@pedropombeiro
Copy link
Collaborator

Interestingly, in this PR, I get the issues fixed in #36, as well. Didn't see them earlier...

Ah probably forgot to remove it from this branch before submitting

@PhilippMundhenk
Copy link
Owner Author

PhilippMundhenk commented Sep 22, 2024

Interesting combination of errors: As the files where not deleted from /tmp (fixed in this PR), the kill didn't actually delete the front pages and folder, thus the rear page scan could progress as intended. Only after this fix, the other error showed up. Sometimes, two wrongs do make a right... ;)

Scans work via flatbed & buttons, ADF & web. Should be sufficient testing. ready to merge. @pedropombeiro : Would you do the honors?

Copy link
Collaborator

@pedropombeiro pedropombeiro left a comment

Choose a reason for hiding this comment

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

Thanks for testing this @PhilippMundhenk

Side note: If it weren't for losing the ability to easily override the scripts, I'd be tempted to rewrite the scripts in something like Go, to make the logic easier to reason about, change, and test. IMHO Bash is too complicated for scripts like this (I understand this is how Brother recommends doing things with brscan4).

@pedropombeiro pedropombeiro merged commit 56b6ab1 into master Sep 22, 2024
1 check passed
@PhilippMundhenk PhilippMundhenk deleted the fixfeat_tempDir_deviceName branch September 22, 2024 18:57
@PhilippMundhenk
Copy link
Owner Author

Indeed. This project has grown. The scripts used to be a hand full of lines and easy to handle. Supporting all kinds of new features as we do now really is not great in this setting. I was thinking of rewriting this, but frankly, I'm lacking time and motivation right now. I would likely use python though, which keeps the ability to override and easy adaptions, while making things much more handelable. The scripts would then remain empty shells calling (potentially a single) python script, handling all features much more easy to maintain.

@PhilippMundhenk PhilippMundhenk mentioned this pull request Sep 22, 2024
@pedropombeiro
Copy link
Collaborator

Yeah, Python would be a good choice. I might take a stab at it at some point.

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.

2 participants