[BUG] STM32_UID still broken, the serial number its is prining out is garbage.

ellensp commented 9 months ago

Did you test the latest `bugfix-2.1.x` code?

Yes, and the problem still exists.

Bug Description (Updated)

If you enable HAS_STM32_UID on a test device It now prints out a serial number, but the number is not correct

First a red hearing, the old code did not give correct results, see notes below. So comparing old results to current results will not help

But the output is still incorrect

On my hardware the Device electronic signature is the following 0x004d0030 0x4646500a 0x20353631

If you enable STM32_UID_SHORT_FORM so the UID is formatted the same as the hex dump above

using current code SERIAL_ECHO(hex_long(UID[0]), hex_long(UID[1]), hex_long(UID[2])); results in 203536312035363120353631 <- incorrect values

expanding out the code to multiple SERIAL_ECHO lines

      SERIAL_ECHO(hex_long(UID[0]));
      SERIAL_ECHO(hex_long(UID[1]));
      SERIAL_ECHO(hex_long(UID[2]));

results in 004D00304646500A20353631 <- correct result

With STM32_UID_SHORT_FORM disabled you get similar results

long form of code, with corrected indexes

      SERIAL_ECHO(F("CEDE2A2F-"), hex_word(UID[1]));
      SERIAL_ECHO(C('-'), hex_word(UID[0]));
      SERIAL_ECHO(C('-'), hex_word(UID[3]));
      SERIAL_ECHO(C('-'), hex_word(UID[2]));
      SERIAL_ECHO(hex_word(UID[5]));
      SERIAL_ECHO(hex_word(UID[4]));

results in CEDE2A2F-004D-0030-4646-500A20353631 <- correct result

but the index corrected condensed version

      SERIAL_ECHO(
        F("CEDE2A2F-"), hex_word(UID[1]), C('-'), hex_word(UID[0]), C('-'), hex_word(UID[3]), C('-'),
        hex_word(UID[2]), hex_word(UID[5]), hex_word(UID[4])
      );

results in CEDE2A2F-3631-3631-3631-363136313631 <- incorrect values

Bug Timeline

recent

Expected behavior

Correct serial number should be reported

Actual behavior

you effectively get a random repeating numbers

Steps to Reproduce

define HAS_STM32_UID 1

hack in this code into M115 to display correct UID

SERIAL_ECHOPGM(" Correct UUID:");
uint32_t *uid_address = (uint32_t*)UID_BASE;
for (uint8_t i = 0; i < 3; ++i) {
  const uint32_t UID = uint32_t(*(uid_address));
  uid_address += 1U;  
  for (int B = 24; B >= 0; B -= 8) print_hex_byte(UID >> B);
}

compile and upload
send a M115 and note serial number difference

Version of Marlin Firmware

Bugfix-2.1.x

Don't forget to include

test Configuration.zip

Crazy-Charles commented 9 months ago

Even using the old way the serial number doesn't match my STM32F103 UID. If i read the 12 bytes at location 0x1FFFF7E8 with the data width set at 32-bit, i get 05DAFF34 35325350 51186321 (the actual UID) The results using M115 CEDE2A2F-FFFF-FFFF-FFFF-FFFFFFFFFFFF CEDE2A2F-05DA-FF34-00FF-9768FFFFFFFF OLD UUID:05DAFF3400FF9768FFFFFFFF

00FF9768 is located at 0x1FFFF7F8 FFFFFFFF is located at 0x1FFFF808

ellensp commented 9 months ago

Your right... its even more broken than I thought.

Documenting for anyone reading:

For my processor stm32f401RE the Device electronic signature register address is 0x1FFF7A10 according to https://www.st.com/resource/en/reference_manual/rm0368-stm32f401xbc-and-stm32f401xde-advanced-armbased-32bit-mcus-stmicroelectronics.pdf page 839

I connect up my stlink and get it working under platformio

If I ask GDB to dump these 3 addresses with

x/3xw 0x1FFF7A10

I get

0x1fff7a10: 0x004d0030 0x4646500a 0x20353631 which does not match the old code

OLD code UUID:004D00300200C000445243F5

or the current code

2035-2035-2035-203520352035

or the new code I was playing with..

Taking a closer look

If we do a larger memory dump

x/16xw 0x1FFF7A10
0x1fff7a10: 0x004d0030  0x4646500a  0x20353631  0xc000fcc0
0x1fff7a20: 0x0200c000  0x87ff08a8  0x05d2f000  0x04ad039c
0x1fff7a30: 0x445243f5  0xffffffff  0x00ffffff  0xc738c639
0x1fff7a40: 0xc53ac738  0xc53ac53a  0xc639c639  0xffffffff

and examine the old code output UUID:004D0030 0200C000 445243F5

first 4 bytes 004D0030 is correct next 4 bytes 0200C000 is at 0x1fff7a20, from wrong address next 4 bytes 445243F5 is at 0x1fff7a30, from wrong address

So the old results where also wrong.

ellensp commented 9 months ago

So the long working version should be

  SERIAL_ECHO(F("CEDE2A2F-"), hex_word(UID[1]));
  SERIAL_ECHO(C('-'), hex_word(UID[0]));
  SERIAL_ECHO(C('-'), hex_word(UID[3]));
  SERIAL_ECHO(C('-'), hex_word(UID[2]));
  SERIAL_ECHO(hex_word(UID[5]));
  SERIAL_ECHO(hex_word(UID[4]));

which prints correct results CEDE2A2F-004D-0030-4646-500A20353631

But the single line version

  SERIAL_ECHO(
    F("CEDE2A2F-"), hex_word(UID[1]), C('-'), hex_word(UID[0]), C('-'), hex_word(UID[3]), C('-'),
    hex_word(UID[2]), hex_word(UID[5]), hex_word(UID[4])
  );

Still produces scrambled result UUID:CEDE2A2F-3631-3631-3631-363136313631

ellensp commented 9 months ago

got it.

It is hex_word, this returns &_hex[byte_start + 2]; but _hex is its own buffer that gets reused

so in serial_echo the parameters hex_word(UID[1]), hex_word(UID[0]), hex_word(UID[3]), hex_word(UID[2]), hex_word(UID[5]), hex_word(UID[4]) all turn into &_hex[byte_start + 2]

so when SERIAL ECHO prints them it just print out the last contents of _hex[byte_start + 2] over and over.

github-actions[bot] commented 7 months ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

MarlinFirmware / Marlin