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

App submission: scanservjs #1095

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

highghlow
Copy link
Contributor

Request: #1085

This app will require the scanner to be connected by usb to the Umbrel. Less that ideal, but that's what usb ports on the Umbrel home are for... right?

Also, btw, can you give me the power to close issues in the umbrel repo? There're a lot of resolved issues where the authors are not responding.

App Submission

App name

scanservjs

256x256 SVG icon

(Submit an icon with no rounded corners as it will be dynamically rounded with CSS. GitHub doesn't allow uploading SVGs directly, so please upload your icon to an alternate service, like https://svgur.com, and paste the link below.)
We will help finalize this icon before the app goes live in the Umbrel App Store.

This project doesn't have an icon. There're no google image results for scanservjs. You'll have to make an icon of your own.

Gallery images

(Upload 3 to 5 high-quality gallery images (1440x900px) of your app in PNG format, or just upload 3 to 5 screenshots of your app and we'll help you design the gallery images.)
We will help finalize these images before the app goes live in the Umbrel App Store.

image

image

There's not much about scanservjs on the internet

I have tested my app on:

I haven't tested this app. Both because I don't currently have access to my server and because I don't have a scanner. But it should hopefully work.

@sund3RRR
Copy link

Probably it will not work, at least for hp scanners.
There are different targets in Dockerfile, you can view build instructions here
Ideally, it is better to make a choice in umbrel store to install basic image or hp/airscan image, but as I understand, it is impossible.

And I think you also need to map usb device /dev/bus/usb/

@highghlow
Copy link
Contributor Author

And I think you also need to map usb device /dev/bus/usb/

They said in the docker guide that --priveleged is used for this

@sund3RRR
Copy link

It can work without privileged mode. I installed libsane-hpaio in the container and mapped usb device, it works at least for hp scanner.

@highghlow
Copy link
Contributor Author

It can work without privileged mode. I installed libsane-hpaio in the container and mapped usb device, it works at least for hp scanner.

But we don't want every usb device to be mapped to scanservjs (And we don't which specific device to mount)

@sund3RRR
Copy link

It can work without privileged mode. I installed libsane-hpaio in the container and mapped usb device, it works at least for hp scanner.

But we don't want every usb device to be mapped to scanservjs (And we don't which specific device to mount)

Hard question. I think grant container privileged rights is worse than mapping the whole usb bus. At least for ubmrel store application.

@highghlow
Copy link
Contributor Author

Hard question. I think grant container privileged rights is worse than mapping the whole usb bus. At least for ubmrel store application.

Other apps (and umbrel itself) might want to use usb

@nmfretz
Copy link
Contributor

nmfretz commented Jun 3, 2024

Thanks for submitting scanservejs @highghlow! And thanks for your input @sund3RRR, much appreciated.

We'll need some help from the community to properly test this app because both @highghlow and I don't have a scanner. This may require some back and forth here. @sund3RRR do you have a SANE compatible scanner?

As I understand it, the idea here is that for this packaging of scanservjs we will assume that a user will plug their scanner directly into their umbrelOS device.

They said in the docker guide that --priveleged is used for this

The increased container privileges may indeed be enough in this case because we have also mapped /var/run/dbus; however, we need to test that this is actually the case.

I have committed some changes to get scanservjs to install correctly and have added what I assume would be the maximum permissions we might need to give this container:

This should allow it to work, but our goal now should be to reduce permissions as much as possible without reducing the functionality of scanservjs. We should test the following:

  1. Does this PR work as of 1bc5437 with with privileged: true, /var/run/dbus. and /dev?

  2. Remove the /dev bind mount. Can you boot the scanservjs container without a scanner plugged in, then plug in a scanner and have it be discovered?

  3. Keep the /dev bind mount but remove privileged: true. Does it still work?

  4. remove both /dev mapping and privileged: true. Does it still work?

  5. If it turns out that the /dev bind mount is required in the testing above, does reducing the mounts scope to /dev/bus/usb still work?

@nmfretz
Copy link
Contributor

nmfretz commented Aug 29, 2024

@sund3RRR bumping this in case you are able to help test.

@nmfretz
Copy link
Contributor

nmfretz commented Aug 29, 2024

@sharknoon do you have a scanner to test by chance?

@sund3RRR
Copy link

I don't have an umbrel anymore because I moved my server on orange pi 5 machine.

@nmfretz
Copy link
Contributor

nmfretz commented Aug 29, 2024

Ah no worries, thanks for the response @sund3RRR

@sharknoon
Copy link
Contributor

@nmfretz Unfortunately I don't have a scanner at the moment, mine broke just a couple of weeks ago... But I will buy a new one soon, mainly because of german bureaucracy 😅

@nmfretz
Copy link
Contributor

nmfretz commented Sep 5, 2024

But I will buy a new one soon, mainly because of german bureaucracy 😅

Ha! Sounds good @sharknoon.

@nmfretz nmfretz marked this pull request as draft November 8, 2024 00:44
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.

4 participants