Ways to disable backspace char escaping?

[Kazurin-775]

In one of my own projects, I had a case where binary data is transmitted through UART. But later I discovered that the code in this crate silently escapes the backspace character \x08 to \x08\x20\x08, which causes data corruption in my code:

match data {
    8 | 0x7F => {
        wait_for!(self.line_sts().contains(LineStsFlags::OUTPUT_EMPTY));
        self.data.write(8);
        wait_for!(self.line_sts().contains(LineStsFlags::OUTPUT_EMPTY));
        self.data.write(b' ');
        wait_for!(self.line_sts().contains(LineStsFlags::OUTPUT_EMPTY));
        self.data.write(8)
    }
    [...]
}

Maybe this behavior should be clearly described in the documentation, and another method which disables escaping should be added?

[phil-opp]

Thanks for reporting! It looks like this escape is there since the very first version of this library: https://docs.rs/crate/uart_16550/0.1.0/source/src/lib.rs . I didn't write the original code so I'm not sure why the escaping was added, perhaps you remember @lachlansneff?

Given that the API operates on raw u8 bytes and might also be used to send non-ASCII data, I don't think that this escaping is a good idea. Even with ASCII data, you might want to send a backspace or bell character as-is. So I'm inclined to just remove this escaping altogether, but only if it doesn't break too many people. We should at least look through the crates.io crates that depend on uart_16550 for this.

[olivercalder]

I just ran into a similar problem while trying to use the serial port to send binary data. It appears that the reason for the special case for 0x08 and 0x7F is to allow terminals to correctly display backspace and delete characters. In particular, the cursor must be moved to the previous character (0x08), then that character has to be erased (in this case by printing a space over it using 0x20) and then, since printing the space character advanced the cursor again, it must again be moved to the previous character using 0x08 again.

In order to maintain backwards compatibility, I think it would be useful to have an accompanying send_raw() function which doesn't have the special case for 0x08 and 0x7F. I just submitted PR #21 which adds such a function.

[Kazurin-775]

Closing this in favor of #21.

[phil-opp]

Thanks for looking into this @olivercalder.

Sounds like the escaping might be desired in some cases. Maybe we should rename send to e.g. send_escape or send_terminal_ecape to make things more clear? We could keep the send function as a deprecated alias to avoid breakage and then remove it in the next breaking version. What do you think?

[olivercalder]

Thanks for merging the changes from PR #21, @phil-opp.

Yes, I agree that it would be good to have explicit functions for sending raw data and data intended to be displayed by a terminal. I also completely agree with what you say regarding the send function.

For naming, I don't have strong opinions, but I'll offer a few suggestions. Another contributor used the name send_byte rather than send_raw, and I think I may prefer the former. Furthermore, I think one could emphasize the distinction between arbitrary data and text data intended for a terminal by perhaps using send_byte (or send_u8) along with send_char, respectively. Alternatively, if the goal was to emphasize the behavior of the functions, rather than the intent, I agree that something like send_raw and send_escaped is preferable.

Personally, I think I prefer emphasizing the intended use case rather than what the function is doing -- that is, send_byte and send_char -- since most people (myself included) will not immediately know what the purpose of the distinction between the two functions is, and it is simple enough to explain what the function does in its documentation. A very similar argument could be made for something like send_raw and send_escaped, however. Do you have a preference?