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

added marlin gcode support and marlin fan pwm #1782

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

SamsonBox
Copy link

I tried to add marlin gcode support. I hope I did not forget any corner case. I also added the fan pwm support.

@arkypita
Copy link
Owner

arkypita commented Mar 9, 2022

Thank you for your code contribution. I see this commit make some modification to general code, so I need time to check it and evaluate impact on other feature. I'll do that asap.

@SamsonBox
Copy link
Author

Hello arkypita,
I looked at the code changes again, and I had the idea to move the code which generated the spindel / laser gcodes to the GrblCore classes. Then there would be a centeral pice of code wich is responsible of than and a lot of duplicate code would be gone. If this would be OK for you, I will close the Request and sand a new one, when ist implemted.

@arkypita
Copy link
Owner

Ok for me! The right way to virtualize for a different controller is to put virtual function in GrblCore and override them in MarlinCore.

Please note that I am not able to test it with marlin, so be care of what you do!

Official LaserGRBL with MarlinCore is used around 300 users (less then 0.5% of lasergrbl users).
I don't know if it work on them, or what else (maybe they edit gcode by hand?) but it is important to not broke anything.
Maybe it is possible to clone a MarlinCore to a "MarlinCoreOld" so the old is keep for compatibility?

@SamsonBox
Copy link
Author

Hello arkypita,
so I moved the generation of the spindle on / off code to the GrblCore an a marlin specific implementation to MarlinCore. I think the changes are now much better compare. So My problem is the other way round. I can cross check grbl changes. But the marlin gcode was not use able on my machine. I found a second tool which post processes the generated code to fit for marlin machines. But I think that should not be necessary. I think my changes are not bullet prof for all use cases (marlin / grbl / M3 / M4 / M5 / M106 / M107). But with the changes we have a good chance to get all the use cases together.

@SamsonBox SamsonBox changed the title added marlin gcode support and marlin fan pwm WIP: added marlin gcode support and marlin fan pwm Mar 12, 2022
@SamsonBox SamsonBox changed the title WIP: added marlin gcode support and marlin fan pwm added marlin gcode support and marlin fan pwm Mar 30, 2022
@SamsonBox
Copy link
Author

SamsonBox commented Mar 30, 2022

@arkypita So I'm finished with my changes and rebased my branch. Can you review the code?

Thank you

@arkypita
Copy link
Owner

arkypita commented Apr 2, 2022

I am a little busy right now but I'll do it asap.

@SamsonBox
Copy link
Author

Hi @arkypita I fixed another merlin bug. Would it be possible to merge this request in the near term? It's quiethard tomerg the generted designer files.

BR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants