mounaiban / captdriver

Driver for Canon CAPT printers
GNU General Public License v3.0
87 stars 16 forks source link

CUPS backend bug #14

Open ra1nst0rm3d opened 2 years ago

ra1nst0rm3d commented 2 years ago

One of your high-priority problems in wiki is CUPS backend bugs. I think, I has fix for this in my repo: just checkout capt_command.c on my repo.

There is diff:

//      while (1) {
//      if (WORD(capt_iobuf[2], capt_iobuf[3]) == capt_iosize)
//          break;
//      if (BCD(capt_iobuf[2], capt_iobuf[3]) == capt_iosize)
//          break;
//      if (cmd == CAPT_NOP || cmd == CAPT_IDENT || cmd == CAPT_CHKJOBSTAT) {
//          capt_recv_buf(capt_iosize, BCD(capt_iobuf[2], capt_iobuf[3]));
//          continue;
//      }
//      /* block at 64 byte boundary is not the last one */
//      if (WORD(capt_iobuf[2], capt_iobuf[3]) > capt_iosize && capt_iosize % 64 == 6) {
//          capt_recv_buf(capt_iosize, WORD(capt_iobuf[2], capt_iobuf[3]) - capt_iosize);
//          continue;
//      }
//      /* we should never get here */
//      fprintf(stderr, "ERROR: CAPT: bad reply from printer, "
//              "expected size %02X %02X, got %02X %02X\n",
//              capt_iobuf[2], capt_iobuf[3], LO(capt_iosize), HI(capt_iosize));
//      capt_debug_buf("ERROR", capt_iosize);
//      exit(1);
//  }

    if(WORD(capt_iobuf[2], capt_iobuf[3]) > 6 || BCD(capt_iobuf[2], capt_iobuf[3]) > 6) {
        capt_recv_buf(capt_iosize, BCD(capt_iobuf[2], capt_iobuf[3]));
    }

Simple, won't it? This implementation fixed buffer mismatch on LBP1120. But, my father detected bug in BCD(). True BCD() is here:

static inline uint16_t BCD(uint8_t lo, uint8_t hi)
{
    uint16_t a, b, c, d;
//      a = (hi >> 8) & 0x0F;
    a = (hi >> 4) & 0x0F;
    b = (hi >> 0) & 0x0F;
//      c = (lo >> 8) & 0x0F;
    c = (lo >> 4) & 0x0F;
    d = (lo >> 0) & 0x0F;
    if (a > 9 || b > 9 || c > 9 || d > 9)
        return WORD(lo, hi);
    return a * 1000 + b * 100 + c * 10 + d * 1;
}
mounaiban commented 2 years ago

@ra1nst0rm3d First, great work on fixing that BCD bug. It looks obvious now it has been pointed out, but it's amazing that nobody else caught it for over 9 years, maybe due to lack of opportunity to get really screwed over by the bug. I didn't really pay attention to it as well...

The hi >> 0 and lo >> 0 looks like it could be removed as well, unless I am missing something related to C standards and compiler behaviour.

I don't think the LBP3000 I use has that bug that causes it to randomly switch to BCD, but I will still give it a go. I have assumed that LBP3000's don't randomly switch to BCD, but I may stand corrected.

While we are still talking about buffers, I have been wanting to try adjusting the size of capt_iobuf to match bytes 10-11 in the response to 0xA1A1, but I don't know if it would help.

ra1nst0rm3d commented 2 years ago

'bout BCD on LBP3k: Anyway, try this with LBP3k. This can work with all types of CAPT printers.

'bout bit-shifting by zero: Maybe, but we need hi & 0x0F anyway :)

mounaiban commented 2 years ago

@ra1nst0rm3d I finally got to test your BCD fix on my LBP3000, and :cursing_face:, it actually worked on one of the problem targets I have been working on, ZorinOS (see #4). I lack the knowledge to explain why it works, but it works.

This time around, I tested it on the 64-bit version of ZorinOS 16.1. Without the BCD fix, the printer gets stuck. After applying the fix, well, it was able to communicate with the printer, like, 10 times in a row even after restarting the printer and the VM. Kudos to you and your dad! :bow:

Could this be the answer to the stuck printer problem we had for almost 8 years? We'll find out! A preliminary fix has been pushed to the mounaiban/0.1.4.1-RE branch, and I will be inviting the rest of the crew at agalakhov#7 to test it out.

I might test this fix with the 32-bit version if I get the time and motivation.

The version I used was as follows:

static inline uint16_t BCD(uint8_t lo, uint8_t hi)
{
    uint16_t a, b, c, d;
    a = (hi >> 4) & 0x0F;
    b = hi & 0x0F;
    c = (lo >> 4) & 0x0F;
    d = lo & 0x0F;
    if (a > 9 || b > 9 || c > 9 || d > 9)
        return WORD(lo, hi);
    return a * 1000 + b * 100 + c * 10 + d * 1;
}
ra1nst0rm3d commented 2 years ago

Glad to hear it)

mounaiban commented 2 years ago

Tested the BCD fix on Zorin OS 15.3 Lite (32-bit), but this time the printer remained stuck. :shrug:

According to the CUPS error log /var/log/cups/error_log the print data made it to the printer, but the driver was unable to get the printer to start printing.

ra1nst0rm3d commented 2 years ago

Maybe, BCD() needs to be separated for 32-bit and 64-bit?

ra1nst0rm3d commented 2 years ago

Like this:

#if __x86_64__ || __ppc64__
static inline uint16_t BCD(uint8_t lo, uint8_t hi)
{
    uint16_t a, b, c, d;
    a = (hi >> 4) & 0x0F;
    b = hi & 0x0F;
    c = (lo >> 4) & 0x0F;
    d = lo & 0x0F;
    if (a > 9 || b > 9 || c > 9 || d > 9)
        return WORD(lo, hi);
    return a * 1000 + b * 100 + c * 10 + d * 1;
}
#else
static inline uint16_t BCD(uint8_t lo, uint8_t hi)
{
    uint16_t a, b, c, d;
    a = (hi >> 8) & 0x0F;
    b = hi & 0x0F;
    c = (lo >> 8) & 0x0F;
    d = lo & 0x0F;
    if (a > 9 || b > 9 || c > 9 || d > 9)
        return WORD(lo, hi);
    return a * 1000 + b * 100 + c * 10 + d * 1;
}
#endif

P.S Checked this with -m32, looks like real solution of our problem.

ra1nst0rm3d commented 2 years ago

Have you tried to uncomment these lines? https://github.com/mounaiban/captdriver/blob/master/src/prn_lbp2900.c#L153-L156

mounaiban commented 2 years ago

@ra1nst0rm3d Which OS did you use? I tried the #if block you posted, but the printer still didn't print.

I didn't expect it to work, but I was hopeful enough to try it. :smile: In versions before 7709565, we have already been shifting by eight bits in BCD(), regardless of bitness or architecture.

As for uncommenting 153-156 in prn_lbp2900.c, I am not willing to do this just yet. That hack was for preventing the LBP3000 from freezing after a job within the same power cycle, different from what we are dealing with here...

I might go through capt-command.c again, this time more thoroughly, to see if we have missed anything. I am especially suspicious of bugs in capt_sendrecv().

ra1nst0rm3d commented 2 years ago

I'm using Arch