Skip to content

added autodetect tape size - #5

Open
Amanuense-del-diavolo wants to merge 9 commits into
Kezii:masterfrom
Amanuense-del-diavolo:master
Open

Amanuense-del-diavolo wants to merge 9 commits into
Kezii:masterfrom
Amanuense-del-diavolo:master

Conversation

@Amanuense-del-diavolo

Copy link
Copy Markdown

added the ability to auto-detect the tape size by interrogating the printer before each print

@marcoSchr marcoSchr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you clean up your changes by checking cargo clippy

Comment thread brother_ql/src/image.rs
fn img_to_lines(img: ImageBuffer<Rgba<u8>, Vec<u8>>) -> Result<Vec<[u8; 90]>, BrotherQlError> {
fn img_to_lines(
img: ImageBuffer<Rgba<u8>, Vec<u8>>,
image_width: u32,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should not need the image width here, as img.width() is the same size

Comment thread brother_ql/src/image.rs
Comment on lines +1 to +2
use std::env;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove the unused import

Comment thread brother_ql/src/image.rs
Comment on lines 96 to +98
for x in 0..img.width() {
let i = img.get_pixel(x, y).0[0];

let x = x + padding;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This can be simplified by using for x in padding..720 {

Comment thread brother_ql/src/driver.rs
/// Get the pixel width (print area width in dots) for the loaded media
pub fn pixel_width(&self) -> Option<u16> {
match (self.media_width, self.media_length) {
// Endless tapes (length = 0) - dots_total - offset_r

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If this is not only the total width, but there is also an offset, would it make sense to include the offset in the return value?

Comment thread brother_ql/src/driver.rs
(58, 58) => Some(688 - 51), // 637

// Unknown media
_ => None,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should be able to capture width and height here and at least log an error or a warning, that we were unable to determine the width.

Comment thread brother_ql/src/image.rs
dithered_img.save("/tmp/out_processed.png")?;

let lines = img_to_lines(dithered_img)?;
// if the paper format is not known, assume the biggest one

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this comment not be part of line 145?

Comment thread brother_ql/src/image.rs

let new_width = 720; //630 per la carta piccola

// let new_width = 720; //630 per la carta piccola

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove this line instead of commenting out

Comment thread printer_bot_rs/Cargo.toml
teloxide-core = "0.13"
thiserror = "2.0.16"
tokio = { version = "1.34.0", features = ["full"] }
openssl = { version = "0.10", features = ["vendored"] }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is there a new dependency for openssl, when it is never used by us directly?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

it's used by the telegram bot

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Then we probably want to put it with the vendored feature behind a feature, as not everyone would want the statically linked openssl

@Kezii

Kezii commented Feb 12, 2026

Copy link
Copy Markdown
Owner

I don't have the mental bandwidth to review this, I'll merge when @marcoSchr approves

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