EdgeTX / edgetx

EdgeTX is the cutting edge open source firmware for your R/C radio
https://edgetx.org
GNU General Public License v2.0
1.6k stars 340 forks source link

strhelper function hex2char() returns wrong characters for 0xa..0xf #3301

Closed mha1 closed 1 year ago

mha1 commented 1 year ago

Is there an existing issue for this problem?

What part of EdgeTX is the focus of this bug?

Transmitter firmware

Current Behavior

radio/src/strhelper.cpp: char hex2char(uint8_t hex) { return (hex >= 10 ? hex - 9 + 'A' : hex + '0'); }

outputs for

  for(uint8_t hex = 0x00; hex < 0x10; hex++)
    printf("hex %x -> \"%c\"\n",hex, hex2char(hex));
hex 0 -> "0"
hex 1 -> "1"
hex 2 -> "2"
hex 3 -> "3"
hex 4 -> "4"
hex 5 -> "5"
hex 6 -> "6"
hex 7 -> "7"
hex 8 -> "8"
hex 9 -> "9"
hex a -> "B"
hex b -> "C"
hex c -> "D"
hex d -> "E"
hex e -> "F"
hex f -> "G"

Expected Behavior

Well, you know ...

Steps To Reproduce

checkout hex - 9

Version

2.8.1 and 2.9

Transmitter

Radiomaster TX16S / TX16SMK2

Anything else?

No response

pfeerick commented 1 year ago

Nice catch. Insidious little gremlin... looks like it was probably due to hex2zchar seemingly doing something similar, but must be handled differently as that is legacy OTX stuff.

mha1 commented 1 year ago

@pfeerick speaking of zchar stuff, isn't all zchar code obsolete?

pfeerick commented 1 year ago

Most, but I'm not sure it's all obsolete - there seem to be some active references to it in some odd places still... like strcatFlightmodeName 🤷 If you want to have a go at killing it once and for all I won't stop you 😁

mha1 commented 1 year ago

I rephrase: shouldn't zchar code be obsolete? The fact zchar code is only rarely used made me suspicious.

Unrelated: I need some advise on another thing I'm implementing and testing right now. Are you available for a discussion on Discord DM? I'll jot down the general idea and some questions. Talk tomorrow or so.

pfeerick commented 1 year ago

It was a pet hate of Raphael's and he is responsible for so little of it existing now ;)

Sure, the next couple of days should be fine. Or you can stop by the dev hour chat thingy that's happening shortly and see what the guys think ;)

mha1 commented 1 year ago

I sympathize.

I'd prefer to have your opinion and advise as a first step if that's not a problem for you.