Serial regression start/end ignored in 2.0, honored in 2.1

[tresf]

Quoting:

After some more testing the issue appears to begin with version 2.1.
Version 2.0.12 works fine.
Serial scale: Avery weigh-tronix 7880

Workaround:

• Remove parameters that are not intended to be used. (e.g. remove width for scales that honor start/end, remove start/end for scales that honor width).

{
  "call": "serial.openPort",
  "promise": {},
  "params": {
    "port": "COM3",
    "options": {
      "start": "0x0002",
      "end": "0x000D",
      "width": "16",
      "baudRate": "9600",
      "dataBits": "7",
      "stopBits": "1",
      "parity": "EVEN",
      "flowControl": "NONE"
    }
  }
}

The issue is unobvious at first, but it's a change in behavior when both start/end AND width are provided. Note, providing both is discouraged, but the old 2.0 API spelled out the behavior, quoting:

Not used if width is provided.

The documentation: @v2.0.7

tray/js/qz-tray.js

Lines 978 to 979 in 28aaefd

	* @param {string} [options.start=0x0002] Character denoting start of serial response. Not used if <code>width</code is provided.
	* @param {string} [options.end=0x000D] Character denoting end of serial response. Not used if <code>width</code> is provided.

This documentation (and apparently the behavior) was removed in 2.1.

QZ Tray 2.0.12 behavior:

• width is honored over start/end

QZ Tray 2.1.3 behavior:

• start/end are honored over width

To reproduce, try this code against qz-tray.js version 2.0.12, and then again against qz-tray.js version 2.1.3:

• Testing against a Mettler Toledo scale, qz-tray.js 2.0.12, QZ Tray 2.1.3, the below code works fine because QZ Tray 2.1.3 prefers start/end.
  • ✅ start/end is honored, the weight is returned from the scale.
• Testing against a Mettler Toledo scale, qz-tray.js 2.0.12, QZ Tray 2.0.12, the below code breaks because QZ Tray 2.1.3 prefers start/end.
  • 🚫 width is honored, the weight isn't returned until W\r is sent twice, and then the weight is returned as a 16-character string, e.g. �000.45 000.45 .

I believe the Avery Weigh-Tronix prefers width, hence the regression in behavior.

var port = "COM3";
qz.websocket.connect().then(() => {
    qz.serial.setSerialCallbacks(function(evt) {
        if (evt.type !== 'ERROR') {
            console.log('Serial', evt.portName, 'received output', evt.output);
        } else {
            console.error(evt.exception);
        }
    });
    var options = {
        start: '0x0002',
        end: '0x000D',
        width: 16,
        

        baudRate: 9600,
        dataBits: 7,
        stopBits: 1,
        parity: 'EVEN',
        flowControl: 'NONE'
    };
    return qz.serial.openPort(port, options);
}).then(() => {
    return qz.serial.sendData(port, "W\r");
}).then(() => {
    return qz.serial.sendData(port, "W\r");
}).then(() => {
    return qz.serial.sendData(port, "W\r");
}).catch(e => {
    console.error(e);
});

This is a pretty specific regression which is introduced by incorrectly providing conflicting parameters, I'm not sure if it's worth writing an upgrade routine for since the upgrade routine could actually cause regressions for other people.

[tresf]

Assigning @bberenz just so he can take a look and help decide how to move forward. My instinct is to do nothing and close as wontfix but I'd like his opinion on it first.

[akberenz]

We could probably support the old behavior when receiving a legacy message by wiping out boundEnd here, but as you pointed out providing both an 'end' and 'width' is already unsupported in both versions so I'm not sure if there's a benefit to adding that in?

[tresf]

Right, I think our window to write an upgrade routine was missed when #442 was merged. Now we're 18 months in with 2.1 and it seems like directing developers to correct the data is the highest level of compatibility (vs. correcting the regression but potentially breaking user space again), despite the fact that we've already broken user-space. Perhaps a warning? (Edit: Done in #829)

[tresf]

Adding a note, that to honor width using the old 2.0 API against 2.1 or higher, you MUST set start/end to a blank string, otherwise, the code will fallback to having default start and end values per the documentation.

This isn't intuitive, but reverting that decision can have other side-effects.