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

Add AnkerMake M5 Profile #13628

Merged
merged 19 commits into from
Dec 2, 2022
Merged

Add AnkerMake M5 Profile #13628

merged 19 commits into from
Dec 2, 2022

Conversation

just-trey
Copy link
Contributor

Hello. I am submitting a well-tested unofficial profile for the new AnkerMake M5 printer. Please review and let me know if there are any corrections needed.

@Raven-935
Copy link

I am new to this whole thing. How do you add this profile to Ultimaker Cura?

@just-trey
Copy link
Contributor Author

I am new to this whole thing. How do you add this profile to Ultimaker Cura?

@Raven-935 until this is merged in, here is a way to do this manually. https://docs.google.com/document/d/15L8_ncIyjsMVN6khWrD168GC-0MIlf46-L7oQHMzbLQ/edit#heading=h.z6ne0og04bp5

@Raven-935
Copy link

Raven-935 commented Nov 2, 2022 via email

@Joeydelarago Joeydelarago added the PR: Printer Definitions 🏭 A PR that introduces or changes settings and printer definitions label Nov 11, 2022
@rburema
Copy link
Member

rburema commented Nov 25, 2022

(Don't worry about the 'publish test results' step, I think that's on our side.)

This is looking good, but you don't have to exclude 2.85mm materials (the UltiMaker ones for example) from a machine that's for 1.75mm -- that already happens internally a step before.

Also, it's usually a good idea to only put things that are actually changed between quality profiles into the quality profiles themselves. If the values are the same, you should probably just put them in the overrides for the machine (or otherwise higher up the chain).

@just-trey
Copy link
Contributor Author

@rburema based on your feedback, I did some digging and realized a ton of stuff I have in there doesn't make it work any better. I am still testing and will be making any adjustments I need. how can I mark this as in progress, so you are not wasting your time reviewing?

@rburema rburema marked this pull request as draft November 27, 2022 10:04
@rburema
Copy link
Member

rburema commented Nov 27, 2022

@just-trey There's a feature just for that: Convert it to a 'draft' pull-request.

Since you don't have write access to this repo though, I do have to do the conversion. (You can make a draft PR as a normal contributor, just not convert a normal PR back to draft, even if it's your own for some reason.)

It'll still show up in the list of open PR's, but the little icon will be grey instead of green, so we know it's not finished.

I'm not sure if you can convert it back, so you might have to message us again to get it back to a 'normal' PR.

@jellespijker
Copy link
Member

jellespijker commented Nov 29, 2022

Can you sync your fork, with our main. We have added a printer-linter tool for our printer definitions and profiles. This should help us with processing our PR's. It performs some standard diagnostic checks and apply auto-formatting on the changed files in this PR, but you can also run it locally. See https://github.com/Ultimaker/Cura/tree/main/printer-linter

If it finds issues for which it knows a fix it should post a comment with that suggested fix such that you can easily commit that fix if you're okay with it.
But please use commonsense with the suggestions because the tool is still in the early stages of development

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-tidy found issue(s) with the introduced code (1/1)

resources/definitions/ankermake_m5.def.json Outdated Show resolved Hide resolved
"machine_start_gcode": {
"default_value": "M104 S{material_print_temperature_layer_0} ; set final nozzle temp\nM190 S{material_bed_temperature_layer_0} ; set and wait for nozzle temp to stabilize\nM109 S{material_print_temperature_layer_0} ; wait for nozzle temp to stabilize\nG28 ;Home\nG1 E10 F3600; push out retracted filament(fix for over retraction after prime)"
},
"machine_end_gcode": {
Copy link
Contributor

@github-actions github-actions bot Nov 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ diagnostic-definition-redundant-override ⚠️
Overriding machine_end_gcode with the same value (M104 S0
M140 S0
;Retract the filament
G92 E1
G1 E-1 F300
G28 X0 Y0
M84) as defined in parent definition: fdmprinter

resources/extruders/ankermake_m5_extruder_0.def.json Outdated Show resolved Hide resolved
@jellespijker jellespijker added the PR: Community Contribution 👑 Community Contribution PR's label Nov 30, 2022
@jellespijker jellespijker reopened this Nov 30, 2022
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-tidy found issue(s) with the introduced code (1/1)

resources/definitions/ankermake_m5.def.json Show resolved Hide resolved
just-trey and others added 4 commits December 1, 2022 00:03
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-tidy found issue(s) with the introduced code (1/1)

@Ultimaker Ultimaker deleted a comment from github-actions bot Dec 1, 2022
@Ultimaker Ultimaker deleted a comment from github-actions bot Dec 1, 2022
@jellespijker
Copy link
Member

@just-trey what do you need to mark this PR as ready for review and can we help you with that?

@just-trey just-trey marked this pull request as ready for review December 2, 2022 04:40
@just-trey
Copy link
Contributor Author

just-trey commented Dec 2, 2022

@jellespijker I went ahead and marked as ready. I am sure it works I was just having some other folks test. Since it works let's go ahead with it and if I have future enhancements I can just make a new PR. Thanks for all the help!

@rburema
Copy link
Member

rburema commented Dec 2, 2022

I see the only requested change is the spurious gcode thing. I think we can merge this. Thank you for your contribution!

Devs, see internal ticket CURA-9967 to track QA processes.

@rburema rburema merged commit 1f53112 into Ultimaker:main Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Community Contribution 👑 Community Contribution PR's PR: Printer Definitions 🏭 A PR that introduces or changes settings and printer definitions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants