MMU3 Feature

[eoyilmaz]

Description

This adds the MMU3 feature from Prusa Firmware.

Requirements

No special requirements

Benefits

It is now possible to use MMU3 hardware along with the features like SpoolJoin etc.

Related Issues

• [FR] MMU3 Feature #26523
• [FR] MMU2S Spool Join #17735

[eoyilmaz]

Hi,

Sorry for not dropping a detailed PR message earlier today, I didn't have time for that.

So this is the MMU3 feature from PrusaFirmware. I tested the code heavily (albeit with only one printer) and it is working fine (but, I need to do more tests after I rebased the code to Marlin's HEAD to create this PR, I see lot's of things have changed).

There are lots of things that I need to drop some note to. But if you were following the #26523 I already reported mostly about my progress there, and the code explains itself.

For a short explanation, 95% of the code is migrated from PrusaFirmware and because it is mostly based on Marlin1 (if I'm not misttaken) I had to replace most of the code that are interacting with the rest of the Marlin, i.e. creating menus, querying something from the planner, saving something to the EEPROM etc. othwerwise the code is directly transferred from Prusa firmware.

And becasue the code is somewhere in between PrusaFirmware and Marlin I think there are places that we have room for optimization. (just remembered that I was going to update the format to use 2 spaces, will do it in a minute).

Anyways, I'm re-reading the code in this PR and I'll drop notes to places where I think I need supervision.

[eoyilmaz]

I'm working on the build test errors for mega2560 👍

[eoyilmaz]

Hi there,

I think the code is now ready to be reviewed...

First, let me say that I'm not a C++ developer, or at least not for the last 10 years or so... So there might be (or definitelly will be) some suboptimal things that I added on top of Prusa's code, especially on the mmu2_reporting.cpp I'm very confused with how I need to use the F() or PSTR() and I was not able to modify the code so that it passes the tests both for STM32 and AVR and I've choosen STM32 compatible code over the AVR.

Known Problems & Bugs

• I didn't throughly check the M707/M708 commands (I'll look in to them as a follow up). DONE!
• Enable/Disable MMU menu item is missing, this is fairly easy, the API is there only the menu items are missing, and I might even do it while you are reviewing the code or I can leave it to another follow up PR. DONE!
• Menus can be finicky. If I don't start the MMU3 before or at the same time the printer starts, I can't select anything other than the first 2 items in the menus. I was having menu issues with MMU2 too and I'll definitely look in to this. DONE!
• The MMU2::ReportErrorHookMonitor() needs more work! DONE!
• Full screen error reporting is missing! (In progress...) DONE!

and more things TODO

• Adjusting the Stall Guard settings for the MMU motors are missing in the menus, I'll work on that as another issue.
• feature/mmu3/mmu_hw/errors_list.h can definitelly be merged to language_en.h but I'm not experienced enough to do it as the Prusa developers are getting the error code from MMU3 as an uint16_t and converting it to a static const char[] PROGMEM by using a dictionary, there are helper functions to do that... You guys would probably do it in 5 minutes or so...

and I think that's all... I really enjoyed working on this, and I wanted to migrate this code to Marlin at the moment Prusa announced it... I was not sure if I was ever be able to do it, and I'm really proud of myself that I was and I'm really greateful both to the Prusa and Marlin community and developers that allowed me to run an MMU3 on my Ender 3 Pro... You guys are great 👍

[eoyilmaz]

Hi @thisiskeithb, do you need anything for this to be reviewed/merged?

[thisiskeithb]

Hi @thisiskeithb, do you need anything for this to be reviewed/merged?

Thinkyhead will do a review once you're done making changes & he has time. There are quite a few other open PRs and there's still prep going on for the next release.

[rerickson1]

Just some friendly feedback. You shouldn't merge the bugfix-2.1.x branch into this PR. You should be doing a git rebase to rebase your changes onto that branch when you want to "pull in" updates. This PR should only contain your commits for the new feature.

[eoyilmaz]

Just some friendly feedback. You shouldn't merge the bugfix-2.1.x branch into this PR. You should be doing a git rebase to rebase your changes onto that branch when you want to "pull in" updates. This PR should only contain your commits for the new feature.

Actually I was trying to do a rebase to update some of my commit messages, and I think I messed up there... I believe I can update the PR to clean it, if desired. I see that I can't update the PR to point to a new branch of mine.

[eoyilmaz]

Just some friendly feedback. You shouldn't merge the bugfix-2.1.x branch into this PR. You should be doing a git rebase to rebase your changes onto that branch when you want to "pull in" updates. This PR should only contain your commits for the new feature.

Is there any easy way to fix this at this stage? I tried doing a rebase again and force pushed to my branch, but I think it didn't work as we want it to be.

and also if you look in to the files changed, you'll see only the files that I changed, nothing else.

[thisiskeithb]

Is there any easy way to fix this at this stage?

I dropped all the extra/duplicate commits & did another force push.

duplicates:

[eoyilmaz]

Is there any easy way to fix this at this stage?

I dropped all the extra/duplicate commits & did another force push.
duplicates:

aah thank you @thisiskeithb 👍

[thinkyhead]

Is there any easy way to fix this at this stage?

git fetch upstream
git merge upstream/bugfix-2.1.x
git reset upstream/bugfix-2.1.x
git add .
git commit -m "MMU3 Feature"
git push -f

[thinkyhead]

The first hour or two of the review and prep time for this PR will be spent adding the standard Marlin header (with © 2024) to all the new source files and reformatting the code according to our standards (i.e., by using the included "uncrust" script and then following up with some adjustments). It would be a great help if you would go ahead and do this yourself so that we don't have to. At that point it will be "ready for review."

[rerickson1]

@eoyilmaz FYI, I am in the process of trying out this PR on my printer. Don't let me hold anything up, just wanted to let everyone know.

[eoyilmaz]

The first hour or two of the review and prep time for this PR will be spent adding the standard Marlin header (with © 2024) to all the new source files and reformatting the code according to our standards (i.e., by using the included "uncrust" script and then following up with some adjustments). It would be a great help if you would go ahead and do this yourself so that we don't have to. At that point it will be "ready for review."

sure sure, this is my first PR to Marlin at this scale... I'll check the contribution guides again, and please do not spend extra time on aything that I can do 👍

[rerickson1]

Should these values added together represent the distance from the extruder gears to the nozzle tip? Maybe we can clarify this with comments?

[eoyilmaz]

I think I moved them up from around line 4540 where a comment exists already. But yeah a comment would be more descriptive.

[rerickson1]

Is this the distance from the filament sensor to the gears?

[eoyilmaz]

Exactly, and it accepts negative values too (considering your setup it will be beneficial for you).

[eoyilmaz]

adding the standard Marlin header (with © 2024) to all the new source files and reformatting the code according to our standard

@thinkyhead as I see, the "uncrust" script adds the Marlin header to all the files (I'll update to 2024), and it contains similar text to the one that exists in: https://github.com/prusa3d/Prusa-Firmware/blob/MK3/Firmware/Firmware.ino

Do we need to add anything to that as these files are coming from PrusaFirmware? Or is this enough?

[eoyilmaz]

The first hour or two of the review and prep time for this PR will be spent adding the standard Marlin header (with © 2024) to all the new source files and reformatting the code according to our standards (i.e., by using the included "uncrust" script and then following up with some adjustments). It would be a great help if you would go ahead and do this yourself so that we don't have to. At that point it will be "ready for review."

@thinkyhead all are done 👍

[thisiskeithb]

I've rebased this PR to bring it up to date with current bugfix-2.1.x & resolved merge conflicts, so please update your local copy with the following before pushing additional commits:

git checkout bugfix-2.1.x_mmu3_pr
git fetch origin
git reset --hard origin/bugfix-2.1.x_mmu3_pr

[thinkyhead]

I'll have this merged shortly for wider testing. Feel free to submit updates to clean up or improve anything, or to add MMU3 menu items for other LCD controllers. In general any values stored in EEPROM that are associated with a G-code should be reported as such so that users can replay that G-code to recover settings. And, if MMU3 settings exist which are stored to EEPROM but which don't have an associated G-code, then a G-code and/or parameters should be created for each of those settings. We will then document those G-codes and other firmwares can choose to adopt them, or not.