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

Wait for finish to send end_print #12

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

glaserL
Copy link

@glaserL glaserL commented Feb 23, 2024

First, thanks a lot for your work!
I recently bought a B21 and .. well, want to make it more complicated than simply installing an app!

I noticed the same behaviour as in #3 and realized increasing the sleep duration also helped.

Somehow I got the exception below, when calling get_print_status. Unsure if this is related to using usb? So I had to modify it a bit.

/niimprint/niimprint/printer.py", line 299, in get_print_status
    page, progress1, progress2 = struct.unpack(">HBB", packet.data)
                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
struct.error: unpack requires a buffer of 4 bytes

Right now it simply prints Progress: VALUE, if you want that changed or have it send to logger i can change that of course.

Looking forward to your feedback!

@AndBondStyle
Copy link
Owner

Thanks, I'll try to find some time on the next week to finally fix this. The problem is that I only have a B21 at hand, and I'm afraid the modification you're suggesting can (for example) break D11/D110 instead. The only way I can be sure my code works is to buy every single one of niimbot printers 🥲

@Orhideous
Copy link

I'm afraid the modification you're suggesting can (for example) break D11/D110 instead.

I have D110 so I can test this changes. «Be not afraid», yeah.

@AndBondStyle
Copy link
Owner

Awesome! Did you test your PR on D110 already?

@Orhideous
Copy link

Which one?…

@AndBondStyle
Copy link
Owner

@Orhideous oh, sorry, I confused you with this PR author. Anyway, testing is appreciated :)

Comment on lines 297 to 301
return {
"finished": finished,
"progress": progress,
"error": error,
}

Choose a reason for hiding this comment

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

Let's made this strongly typed? Like, def get_print_status(self) -> Progress:, and

@dataclass
class Progress:
    finished: int  # or whatever

@glaserL
Copy link
Author

glaserL commented Feb 23, 2024

Thanks for the fast response!
You are right,
#3 (comment) mentioned a difference between whole job progress and label progress but I'm not sure how to print multiple labels at once so I didn't take it into account - this might make the progress logging behave weird.

If you don't have time for extensive testing you can also just c+p a few logs and I try to make sense of them!

pyproject.toml Outdated
@@ -1,6 +1,6 @@
[tool.poetry]
name = "niimprint"
version = "0.1.0"
version = "0.1.1"
Copy link

Choose a reason for hiding this comment

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

I don't think this is the right place for bumping a version - that should probably be a separate commit with a GitHub release attached to it

@glaserL
Copy link
Author

glaserL commented Apr 4, 2024

Hey, thanks for all the feedback!

sorry life happened, I'll fixup this PR on the weekend and try to have a look at #17

TheZoc added a commit to TheZoc/niimprint that referenced this pull request Apr 23, 2024
- Added basic error checking before attempting to print (Open paper compartment and busy)
- Unpacking on get_print_status() now works. Tagged unknown values for the time being.
- Added two unknown values to InfoEnum
@jhnnyb
Copy link

jhnnyb commented May 28, 2024

Probably should have taken a better look at the open PRs before making mine. Looks like this PR address the problem I patched. @glaserL is there any update with this PR?

@glaserL
Copy link
Author

glaserL commented May 28, 2024

No, unfortunately not. I know I promised an update weeks ago, next week should (seriously) work out, not sure if it'll be Monday. What printer do you have? I think we already have access to B21 and a D110 so we can make sure we don't break other printers.

@jhnnyb
Copy link

jhnnyb commented May 28, 2024

No issue, there isn't any rush. I have a B1, so I can help test compatibility.

@AndBondStyle
Copy link
Owner

sorry life happened

Same story... Can't get myself to sit down and fix all accumulated issues

Anyway, I'm pretty sure that as-is, this PR will break D110 compatibility. @Orhideous if you're available, can you please test this?

@jhnnyb
Copy link

jhnnyb commented May 28, 2024

No worries, such is life. The utility still has been very useful for making custom stickers! I might have access to a D110, if @Orhideous isn't available I can see if I can get my hands on it to test on.

@glaserL
Copy link
Author

glaserL commented Jun 15, 2024

Good lord! Finally got around to it, sorry for the insane delay.
@AndBondStyle you mentioned it breaks D110, why exactly? I added the changes from @azplanlos in #28 , is that sufficient?

@TheZoc TheZoc mentioned this pull request Jul 26, 2024
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.

5 participants