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

Switch to pure PyUSB interface #56

Merged
merged 9 commits into from
Jun 29, 2023
Merged

Switch to pure PyUSB interface #56

merged 9 commits into from
Jun 29, 2023

Conversation

maresb
Copy link
Collaborator

@maresb maresb commented May 28, 2023

Closes #51

This should drastically simplify everything if it works. It may even lead to platform-independence! (MacOS + Windows)

Currently we're using primarily the raw HID file interface. In parallel we have an experimental PyUSB interface.

There's a lot of effort in maintaining these two separate interfaces. Also, the old HID file interface requires the awkward usb_modeswitch utility. PyUSB should make things nicer.

Please also try experimenting with removing the usb_modeswitch config. Same with the udev config, since the program should also provide custom instructions for configuring udev permissions.

Please test this and provide feedback here. Remember to include the model. I made the output rather verbose, so this should be helpful in diagnosing any issues. Thanks!!!

Install command:

pipx install --pip-args=pre --force dymoprint==2.0.0b0
# OR
pipx install --force git+https://github.com/computerlyrik/dymoprint@pyusb-only

Revert to release version:

pipx install --force dymoprint==1.4.1

@maresb maresb force-pushed the pyusb-only branch 2 times, most recently from 79bd8dc to 6cbbdf2 Compare May 28, 2023 17:26
@maresb maresb mentioned this pull request May 28, 2023
@nathancatlow
Copy link

Sorry for the delay in testing this, works absolutely fine as far as I can see on the PnP printer, great job!

The only thing I had to change was the udev rule, my arch system did not have a 'plugdev' group, and it advises to use this (see Allowing_regular_users_to_use_device);

ACTION=="add", SUBSYSTEMS=="usb", ATTRS{idVendor}=="0922", ATTRS{idProduct}=="1001", MODE="0660" TAG+="uaccess"

I did need to reboot though, resetting udev didn't seem to work. After that, flawless so far.

Thank you for your efforts, this version looks amazing.

@maresb
Copy link
Collaborator Author

maresb commented Jun 8, 2023

Wonderful, thanks so much @nathancatlow for the feedback!!! I'll make note of your instructions under the Arch section when it comes time to merge.

@maresb
Copy link
Collaborator Author

maresb commented Jun 11, 2023

Hi @nathancatlow, does it work with the following rule?

ACTION=="add", SUBSYSTEMS=="usb", ATTRS{idVendor}=="0922", ATTRS{idProduct}=="1001", MODE="0660", GROUP="plugdev", TAG+="uaccess"

(I inserted GROUP="plugdev" into your rule.) This works under Ubuntu. It would be great to find a rule which works simultaneously for both Ubuntu and Arch.

@maresb
Copy link
Collaborator Author

maresb commented Jun 11, 2023

@nathancatlow, I have a different refresh procedure for udev which should hopefully work across Linux distributions. I hope this will save you from having to reset?

sudo udevadm control --reload-rules
sudo udevadm trigger --attr-match=idVendor="0922"

@maresb
Copy link
Collaborator Author

maresb commented Jun 11, 2023

The latest commit on this branch runs for me in a Windows virtual machine if I set the driver to WinUSB using Zadig.

@tomek-szczesny
Copy link
Contributor

I hope to receive my PNP today and I'll test this branch. I've never used this software before so this should be a representative example of a new user.
Anything in particular you want me to test?

@maresb
Copy link
Collaborator Author

maresb commented Jun 12, 2023

That's very valuable, thanks! Please try installing with just pipx and run dymoprint without following the installation instructions. I hope it will instruct you properly itself, and in a better way.

@tomek-szczesny
Copy link
Contributor

Okay, here's what I've done:

  • Updated my Manjaro
  • installed pipx
  • installed dymoprint using a command with github address in it
  • As per instruction I added the udev rule (had to use some other group that actually exists in my system, I used "lp" as it's for printing anyway). udev refresh procedure seems to work well without restarting the machine.
  • Now dymoprint works like this:
[mctom@Tomusiomat ~]$ dymoprint dupa
Found one Dymo device: <DEVICE ID 0922:1001 on Bus 001 Address 007>
  manufacturer: Dymo
  product: DYMO LabelManager PnP
  serial: 15431630032012
  configurations:
  - <CONFIGURATION 1: 500 mA>
    interfaces:
    - <INTERFACE 0: Human Interface Device>
    - <INTERFACE 1: Mass Storage>

Recognized device as LabelManager PnP (no mode switch)
Active device configuration already found.
Opened HID interface: <INTERFACE 0: Human Interface Device>
Detaching kernel driver from interface 0
Printing label..
Sending chunk of 303 bytes
Traceback (most recent call last):
  File "/home/mctom/.local/bin/dymoprint", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/mctom/.local/pipx/venvs/dymoprint/lib/python3.11/site-packages/dymoprint/command_line.py", line 195, in main
    print_server.print_label(label_bitmap, margin=args.m)
  File "/home/mctom/.local/pipx/venvs/dymoprint/lib/python3.11/site-packages/dymoprint/dymo_print_engines.py", line 493, in print_label
    lm.printLabel(label_matrix, margin=margin)
  File "/home/mctom/.local/pipx/venvs/dymoprint/lib/python3.11/site-packages/dymoprint/labeler.py", line 203, in printLabel
    self.rawPrintLabel(lines, margin=margin)
  File "/home/mctom/.local/pipx/venvs/dymoprint/lib/python3.11/site-packages/dymoprint/labeler.py", line 225, in rawPrintLabel
    response = self.sendCommand()
               ^^^^^^^^^^^^^^^^^^
  File "/home/mctom/.local/pipx/venvs/dymoprint/lib/python3.11/site-packages/dymoprint/labeler.py", line 72, in sendCommand
    rspBin = self.devin.read(8)
             ^^^^^^^^^^^^^^^^^^
  File "/home/mctom/.local/pipx/venvs/dymoprint/lib/python3.11/site-packages/usb/core.py", line 423, in read
    return self.device.read(self, size_or_buffer, timeout)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/mctom/.local/pipx/venvs/dymoprint/lib/python3.11/site-packages/usb/core.py", line 1029, in read
    ret = fn(
          ^^^
  File "/home/mctom/.local/pipx/venvs/dymoprint/lib/python3.11/site-packages/usb/backend/libusb1.py", line 864, in intr_read
    return self.__read(self.lib.libusb_interrupt_transfer,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/mctom/.local/pipx/venvs/dymoprint/lib/python3.11/site-packages/usb/backend/libusb1.py", line 954, in __read
    _check(retval)
  File "/home/mctom/.local/pipx/venvs/dymoprint/lib/python3.11/site-packages/usb/backend/libusb1.py", line 602, in _check
    raise USBTimeoutError(_strerror(ret), ret, _libusb_errno[ret])
usb.core.USBTimeoutError: [Errno 110] Operation timed out

I'll restart my machine and see if it helps.

@tomek-szczesny
Copy link
Contributor

1.4.1 doesn't work for me either so that's probably not related to this discussion. :/

@maresb
Copy link
Collaborator Author

maresb commented Jun 12, 2023

Oh, this is really excellent data!!! I'm glad you're testing this on a fresh setup.

For reference, here's what it looks like for me:

$ dymoprint --version
dymoprint 2.0.0b0
$ dymoprint -m0 ""
Found one Dymo device: <DEVICE ID 0922:1001 on Bus 001 Address 078>
  manufacturer: Dymo
  product: DYMO LabelManager PnP
  serial: 10581529032012
  configurations:
  - <CONFIGURATION 1: 500 mA>
    interfaces:
    - <INTERFACE 0: Human Interface Device>
    - <INTERFACE 1: Mass Storage>

Recognized device as LabelManager PnP (no mode switch)
Active device configuration already found.
Opened HID interface: <INTERFACE 0: Human Interface Device>
Printing label..
Sending chunk of 17 bytes
Post-send response: [64, 0, 2, 100, 13, 199, 0, 0]
Done printing.
Cleaned up.

I am really wondering what's leading to the timeout. It's interesting that the kernel driver is active for you (hence the detach) but not for me. Perhaps that has something to do with it. It's really tough for me to properly debug this stuff because I hit "works for me". 😄

As per instruction I added the udev rule (had to use some other group that actually exists in my system, I used "lp" as it's for printing anyway). udev refresh procedure seems to work well without restarting the machine.

That's really great to know. Did you try it with the nonexistent plugdev group? Basically, my hope was that TAG+="uaccess" should work in case plugdev doesn't exist. (See #56 (comment) and link therein.) But I didn't have any good way to test it.

What's the name of the group on your system? Is it lp? I didn't quite understand that comment.

@tomek-szczesny
Copy link
Contributor

Okay, I got it to work on version 2.0.0. I succeeded after I left it connected to USB charger for some 20 minutes :)
I bought my unit from one of those online sellers who sell returns from other shops. So it was a gamble, no guarantees about the state of the device, but I'm an electronics engineer so chances were I'd fix it. Turns out the battery must have been empty (I'm glad I checked the other issues in this repo).
A €30 gamble well worth it. :)

About udev rules, the default content of a rules file didn't work until I changed the group name in the rules file. I changed it to lp, a default group for printer users. My user account already belongs to lp because I also operate RS232 thermal printer.
I believe there is no special meaning of group "lp", it was just my preference.

If Debian and derivatives also have lp group, that sounds like a reasonable candidate for future udev rule templates.

I'll post some response codes I extracted from my failed attempts.

@maresb
Copy link
Collaborator Author

maresb commented Jun 22, 2023

I was just now flipping through some of the issues, and I noticed here that @MatthiasLohr also uses the lp group.

I wonder if this is a standard, and I wonder if we can detect it. (Maybe we could parse /etc/group or something?)

@MatthiasLohr
Copy link

lp group is somewhat standard for printer devices. But it still can vary, depending on so many factors. Just the existence of the lp group does not mean the device is in there. Or what are you referring to?

@tomek-szczesny
Copy link
Contributor

Just yesterday I installed the same software on my girlfriend's laptop with Linux Mint, also worked fine.
The group plugdev was present so no changes were necessary.
I admit the process is too cumbersome for casual users, and some installation script would be handy.

The script may run command "groups" and see which groups the user belongs to. If there's plugdev or lp it could make an educated choice.

@MatthiasLohr
Copy link

But what would you do then with this information? Without root, you can't do anything about that.

The better, maybe best way, IMO: Use udev. If installed as root, install a udev rule, which raises the privileges of that device (will need to collect a list of VID/PID of the respective devices) so everyone can write to it.

@maresb
Copy link
Collaborator Author

maresb commented Jun 22, 2023

Any ideas on how to make the process less cumbersome? I thought the autogenerated instructions here in v2 were a big step forward.

If we can detect the correct group name, then we can autogenerate the correct instructions for adding the udev rule. Maybe we do something like prioritize plugdev, lp, [group of current user].

I'd prefer to keep dymoprint user-installable, and I'd like to avoid something that directly modifies the udev config without user intervention.

@tomek-szczesny
Copy link
Contributor

tomek-szczesny commented Jun 22, 2023

I don't see it as a big problem that there are no root privileges at the time of installation.

I think the install script (bash?) shoud do the following:

  • Warn the user that sudo will be used by the script and they may be asked for a password
  • Try installing git and pipx using apt (with sudo)
  • if there's no apt, then try pacman
  • if no pacman then (some Fedora user contributes their package manager here)
  • install dymoprint using pipx as usual
  • run groups to detect if plugdev or lp is present and pick one of them.
  • create udev rules file (for all supported printer models, for simplicity) (using sudo again)
  • Apply changes in udev rules
  • Let the user know that reboot may be necessary.

I don't think manipulating udev is a big deal, after all a new file with unique name is being created and this can be reverted easily.

EDIT: I believe that lp is a standard group created by CUPS, the printer manager. I think that most desktop users should be a part of it.

@maresb
Copy link
Collaborator Author

maresb commented Jun 22, 2023

Maybe we could do the trick of curling an install script and piping it to python3.

I think we only need Git for development installs.

On my Ubuntu 20.04 install, I'm in lpadmin but not lp, although lp exists. Maybe we can just add all plausible groups?

@tomek-szczesny
Copy link
Contributor

Frankly, I don't see it as a threat to allow everyone to access DYMO printer, not just users from some specific group, to which everyone belongs anyway. Perhaps we're wasting time trying to restrict access to some specific group.

@maresb
Copy link
Collaborator Author

maresb commented Jun 22, 2023

Agreed. I don't know much about udev, but if there's a simple rule for "all users" then let's use it.

@MatthiasLohr
Copy link

MatthiasLohr commented Jun 22, 2023

What the... sorry guys, aber why bash scripts, pacman, etc...? Hey, it's python! Lets's use pip packages, that is what they have been invented for.

I think it is extremely easy to argue how to solve and then solve it.

  • If you're root during install: Great, just install udev rules.
  • If you're not root during install, let the user know "hey, I'm not root, but I really suggest you to have these udev rules for you, do you want me to create them y/n?" -> yes leads to a sudo command.
  • If you're not root during install, and the user said "no root for you", then print "aaah, please considere these rules, either as printed here or as located in that file there, otherwise it might not work".

End of the magic. Should cover any cases, uses standard Python PIP packes (with some easy to add post installation script, invoked by the setup routing), and each user can decide to install it as root or not, and otherwise grant root privileges for udev or not. No curl, no git, no bash, no pipes.

@MatthiasLohr
Copy link

Agreed. I don't know much about udev, but if there's a simple rule for "all users" then let's use it.

That's what I meant with "raise privieleges". Just allow everyone to access the printer. Unfortunately, I guess, it's one rule per device (identified by usb vendor and product id, short VID/PID).

Example:

# allow everyone to read/write to an USB device with specific vendor/product IDs
ACTION=="add", SUBSYSTEM=="usb", ATTRS{idVendor}=="1234", ATTRS{idProduct}=="wxyz", MODE="0666"

@tomek-szczesny
Copy link
Contributor

The point of bringing up bash and apt was to install pip or pipx in the first place. :)
And while we're at it, the same script might as well install the pip package, and configure udev.

Python surely can do that, but guerilla installation of pip, outside the jurisdiction of a package manager is... regrettable.

With udev simplified down to granting access to anyone, your bullet-point flowchart sounds OK to me.
I think that udev rules files can contain more than one rule, so no problem here. A template file may cover all supported printer models.

@MatthiasLohr
Copy link

The point of bringing up bash and apt was to install pip or pipx in the first place. :)

I would leave that out of configuration, since that covers more than just this project. I actually try to avoid things offering me a longish bash script for installation. Always have the fear that "something" might happen.

Python surely can do that, but guerilla installation of pip, outside the jurisdiction of a package manager is... regrettable.

This leaves the room for the question what is "outside the jurisdiction". But in general, I agrees on that. However, a lot of (per-default installed as root) debian packages bring and install their udev rules in /etc/udev/rules.d/*. Beside that it's pip and not apt, I personally don't see any reason not to do it similar, if it is decided to need udev rules.

I think that udev rules files can contain more than one rule, so no problem here. A template file may cover all supported printer models.

That's correct. Just take my line I posted above and duplicate it per different product id (presumably, vendor id should be same for all?). Then, we could just write the file /etc/udev/rules.d/70-dymoprint.rules (number and name open for discussion), trigger a udev reload, that should be it.

@tomek-szczesny
Copy link
Contributor

If udev rules can be applied by installation script of dymoprint's pip package, then I think it should be done - in this case removal of a package will reverse this change and delete udev rules file, which is perfect. Unfortunately I don't know anything about packaging, be it pip or apt.

I think you got me convinced here. Perhaps the installation should be simple enough so no script shall be needed.

A manual like:

  • make sure you have python3 and pipx installed (maybe an example command for debian-verse)
  • pipx install dymoprint, or whatever (Note: the script might require you to authorize sudo access).

@MatthiasLohr
Copy link

Full ACK on that. I can support with pip packaging if required.

How to proceed on this?

@tomek-szczesny
Copy link
Contributor

Well, the usual workflow is to fork this repository onto your own account, commit changes to it, and then do a pull request back into this repository.

Then a bunch of random people like you and me will debate on it, and hopefully the owner of this repository will accept the change.

@maresb
Copy link
Collaborator Author

maresb commented Jun 22, 2023

I'd like to keep things as simple as possible. I feel like install scripts are a bit of an antipattern, but the problem in this case is how to set up the udev rule while:

  1. keeping this as simple as possible for the user
  2. not doing funny things behind the user's back

and these two are somewhat in conflict.

The current v2 approach is:

  • Have the user install pipx
  • Use pipx to install dymoprint
  • Run the dymoprint command, and in case of a permissions problem, it will suggest a sudo one-liner to the user to add the udev rule

I actually feel like this strikes a decent balance between 1 and 2. And if we modify the current udev rule so that everyone gains access, I think it should be reliable.

Before sinking more time and discussion into scripts, I'm very interested to know specifically what you find cumbersome in this approach.

@tomek-szczesny
Copy link
Contributor

That's a good question. Personally I think it's too hard to notice there is an instruction at the end of dymoprint output, because before that there's 15 lines of error traceback. Most users will just ignore reading that.

There's no instruction available in dymoprint_gui at all, and many users will flock over this tool.

And finally, allowing everyone to access just this class of devices is not a big deal. Isn't that what Windows does all the time?

But I get it, it's not right to change someone's udev rules without saying anything. But in the end, the user will have to type in a sudo password, and will be notified what this is for.

@maresb
Copy link
Collaborator Author

maresb commented Jun 22, 2023

because before that there's 15 lines of error traceback

Ok, but we can catch the exception and print only the instruction.

There's no instruction available in dymoprint_gui at all, and many users will flock over this tool.

Ya, it's not so mature yet.

And finally, allowing everyone to access just this class of devices is not a big deal. Isn't that what Windows does all the time?

Agreed.

But in the end, the user will have to type in a sudo password, and will be notified what this is for.

Ya, and I kind of like that the command is directly tied to the permissions error. If we did this during a setup script, then maybe we add unnecessary rules. And then if something in the permissions change, then in order to fix it, the user want to run the setup script again. Then the setup script becomes some bit of magic.

So... let's suppose we suppress the traceback on the permissions error, and we improve the instruction in dymoprint_gui. Then is there still any reason to do an install script?

@tomek-szczesny
Copy link
Contributor

I think the pip setup script already exists (setup.py), it's a matter of extending it.
dymoprint could indeed catch the error and show the instruction. But that's not ideal for one scenario I could think of.
Suppose that someone has to prepare 5 workstations with such a printer. Not enough workstations to automate install procedures, but enough to forget about something. Normally we assume that after a package is installed it's ready for use. It's too easy to forget there's that one extra step a power user has to take care of (using his sudo privileges).

I would really prefer having that udev rule file added by a setup script, and deleted when the package is removed.

I can't see a reason for changing anything in that udev rule file in the future. It is a separate file that has dymoprint in its name, it is clear what is it for, only root can modify it, the printers' VID and PID will never change.

Rule files have, by convention, a number as a file name prefix. The purpose of that is to assign priority. We could assign it priority 00, and let it be overridden by other rules if there's ever any conflict with preexisting rules.

@tomek-szczesny
Copy link
Contributor

So how about merging this thing already? Nobody said it's not working.

@maresb
Copy link
Collaborator Author

maresb commented Jun 29, 2023

@tomek-szczesny, as you wish 😉

@maresb maresb merged commit 27bf3c2 into master Jun 29, 2023
3 checks passed
@maresb maresb deleted the pyusb-only branch July 7, 2023 14:50
@maresb
Copy link
Collaborator Author

maresb commented Jul 7, 2023

I would veto anything involving setup.py. It's a relic from the past history of setuptools, and modern Python projects are supposed to use pyproject.toml where the package configuration is entirely declarative. With pyproject.toml it's possible to inspect metadata and install a package without arbitrary code execution, and I think this is a very good thing.

This discussion seems to be continuing in #67

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.

Does not work on windows
4 participants