Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] (STM32 UUID Never Replaces Default UUID) #26698

Closed
1 task done
sphawes opened this issue Jan 17, 2024 · 5 comments
Closed
1 task done

[BUG] (STM32 UUID Never Replaces Default UUID) #26698

sphawes opened this issue Jan 17, 2024 · 5 comments

Comments

@sphawes
Copy link
Contributor

sphawes commented Jan 17, 2024

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

Yes, and the problem still exists.

Bug Description

Hey there!

I believe I found a bug with how M115 handles reporting the machine's UUID when trying to pull an STM32's UUID from the chip itself, not from configuration.

This feature was added about a year ago (and starts here in the M115.cpp file). The intention with the feature was to replace Marlin's config-defined UUID with the one found in the STM32 itself if 1) the HAS_STM32_UID flag is set in the board's pins file, and 2) the MACHINE_UUID is not already defined.

However, due to this #ifndef in langauge.h, that second condition never happens because we're always setting a MACHINE_UUID. I was able to get this feature to work by simply commenting out this #ifndef, but I'm assuming it's there for a reason!

I'd love to patch this myself, I'm just curious about how y'all think it's best to fix before submitting a PR. I had two thoughts:

  1. Removing the second constraint (&& !defined(MACHINE_UUID)) for using the STM32 UUID and just printing out a second UUID and renaming that serial print line "STM32 UUID" or something so it doesn't step on the toes of the existing UUID logic, but I start getting bugs with the print_hex_byte() function not being defined. I like this approach because a user-defined UUID doesn't get removed if the STM32 UUID is enabled.
  2. Updating language.h to only define the MACHINE_UUID to equal DEFAULT_MACHINE_UUID if HAS_STM32_UID is not defined as well.

Would love to know what y'all think would be the best approach! Once I know what makes sense, let me know and I'll file a PR. I understand this is not super widely available hardware, so I'm happy to run any tests on my hardware and validate a fix.

Bug Timeline

Not sure, might have been from feature introduction.

Expected behavior

I expected that M115 would report the STM32 UUID instead of the Default UUID when the HAS_STM32_UID flag is set in the board's pins file, and that MACHINE_UUID is not set in the configuration file.

Actual behavior

Instead, it reports the Default UUID value: cede2a2f-41a2-4748-9b12-c55c62f367ff.

Steps to Reproduce

  1. Compile Marlin for OPULO_LUMEN_REV4 with #define HAS_STM32_UID in the pins file, and #define MACHINE_UUID "00000000-0000-0000-0000-000000000000" commented out in Configuration.h. Flash onto LumenPnP REV04 motherboard (or any other STM32 board with the same chipset).
  2. Send M115 to Marlin.
  3. Observe the default UUID coming back in response instead of the STM32's UUID.

Version of Marlin Firmware

bugfix-2.1.x

Printer model

Opulo LumenPnP v3.1

Electronics

Stock LumenPnP REV04 Motherboard

LCD/Controller

None

Other add-ons

None

Bed Leveling

None

Your Slicer

None

Host Software

None

Don't forget to include

  • A ZIP file containing your Configuration.h and Configuration_adv.h.

Additional information & file uploads

Archive.zip

@ellensp
Copy link
Contributor

ellensp commented Jan 19, 2024

Please give https://github.com/ellensp/Marlin/tree/stm32-uuid a try

You can see what I changed here bugfix-2.1.x...ellensp:Marlin:stm32-uuid

Sadly I cannot test on real hardware, don't have a OPULO_LUMEN though I would like to change that...

But I did make it work under the marlin simulator, so it is 3/4 tested.

If this works as you think it should ill turn this in a pr for further scrutiny

@sphawes
Copy link
Contributor Author

sphawes commented Jan 20, 2024

oh cool @ellensp i love how you did that!! I'll see if I can test on real hardware today and report back with results. thank you so much!

@sphawes
Copy link
Contributor Author

sphawes commented Jan 20, 2024

@ellensp Just tested this across a couple LumenPnP REV04 motherboards, works perfectly!

Maybe adding a commented out #define HAS_STM32_UID in Configuration.h underneath the #define MACHINE_UUID with a note would be good for discoverability of the feature? Not sure if it's worth adding though.

@thinkyhead
Copy link
Member

With a PR created to address the issue we can close this one. Thanks for the report!

Copy link

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.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants