robotastic / trunk-recorder

Records calls from a Trunked Radio System (P25 & SmartNet)
GNU General Public License v3.0
827 stars 191 forks source link

fix parse error errors #864

Closed taclane closed 7 months ago

taclane commented 7 months ago

Debug variables s0 and s1 in p25_parser.cc are typed as uint8_t. When displayed in an iostream, these variables will be presented as a character, not a number, since that type is interpreted as an alias for unsigned char.

BOOST_LOG_TRIVIAL(error) << "P25 Parse error, s: " << s << " s0: " << s0 << " s1: " << s1 << " shift: " << shift << " nac: " << nac << " type: " << type << " Len: " << s.length();

This console message can display when conditions are bad or other receiver problems occur, and it does not seem ideal to inject invalid, or random, UTF8 characters into the user's console or log files. Additional examples of this can also be seen in #775.

[2023-11-13 15:14:18.618786] (error)   P25 Parse error, s:  s0:
 s1: � shift: 0 nac: 129 type: 65535 Len: 0

Using a uint16_t or short instead should avoid this in the future, and numerically display values for those variables.

Edit: An example of a "corrected" parse error message is below:

[2023-11-13 22:24:17.747166] (error)   P25 Parse error, s:  s0: 0 s1: 97 shift: 0 nac: 97 type: 65535 Len: 0
robotastic commented 7 months ago

That looks much better - thanks!

tadscottsmith commented 7 months ago

FWIW - I've noticed that if the signal is bad enough that this message is displayed, most of the variables in that message are just garbage.

taclane commented 7 months ago

FWIW - I've noticed that if the signal is bad enough that this message is displayed, most of the variables in that message are just garbage.

That is my experience too. You almost never see the message when things are working correctly. But the times it does show up probably should not dump random Unicode or unsanitized control characters to console.

taclane commented 7 months ago

After checking back on things, #870 is the correct fix for this issue. Changing the type declaration has unhelpful side-effects.