slic3r / Slic3r

Open Source toolpath generator for 3D printers
https://slic3r.org/
GNU Affero General Public License v3.0
3.29k stars 1.29k forks source link

New Marlin acceleration M204 syntax #2745

Closed thiagopeixoto16 closed 8 years ago

thiagopeixoto16 commented 9 years ago

Now on, Marlin will have a different syntax for M204 command. I'm not sure if they are in contact with you but the new syntax is: M204 P# T# R# , where: P = Printing moves R = Retract only (no X, Y, Z) moves T = Travel (non printing) moves From: http://www.reprap.org/wiki/Gcode#M204:_Set_default_acceleration

I'm concerned about these changes and how they'll affect the g-code generation from Slic3r. Marlin is starting a new phase with a lot of new concepts and developers so I'd like to give the suggestion to create a dedicated G-code flavor for Marlin at Printer Settings session in order to accommodate any new g-code change from their side. image

Thanks again for this great software!

alranel commented 9 years ago

Thanks for the heads-up. I dislike this unneeded backwards-incompatible change.

Marlin documentation still lists the old syntax: https://github.com/MarlinFirmware/Marlin/blob/Development/Documentation/GCodes.md

A dedicated G-code flavor would need to be called "new Marlin" or "Marlin 2" or something, as all of the tens of thousands of printers running the legacy firmware out there will still need the previous syntax. Do you know whether this syntax change for M204 can be tied to any versioning?

alexborro commented 9 years ago

@alexrj , it was needed in order to change the M204 functionality. From now on, we have separate acceleration for Printing moves and Traveling (non-printing) moves. I have just updated the Marlin documentation - I did it in the wiki and forgot the Marlin docs.

If you have any further question, let me know.

Cheers.

Alex.

alranel commented 9 years ago

@alexborro, I'd still keep the "S" as a shortcut for setting both "P" and "T". This would allow some retrocompatibility.

What's the current versioning scheme? Is this change being introduced along with a major version change, so that users can identify whether they're using an old Marlin or a new one?

alexborro commented 9 years ago

Done.

alranel commented 9 years ago

Thank you for restoring "S" as a legacy shortcut. I'm thinking of a way to handle this syntax change as there not seem to be a versioning change on your side.

Maybe Slic3r could output M204 Sx Px. Older Marlin will ignore the P argument. New Marlin should ignore S if P is supplied. So I would do this in your code:

inline void gcode_M204() {
  if (code_seen('P'))
  {
    acceleration = code_value();
    SERIAL_ECHOPAIR("Setting Printing Acceleration: ", acceleration );
    SERIAL_EOL;
  }
  else if (code_seen('S'))   // Kept for legacy compatibility. Should NOT BE USED for new developments.
  {
    acceleration = code_value();
    travel_acceleration = acceleration;
    SERIAL_ECHOPAIR("Setting Printing and Travelling Acceleration: ", acceleration );
    SERIAL_EOL;
  }
  if (code_seen('R'))
  {
    retract_acceleration = code_value();
    SERIAL_ECHOPAIR("Setting Retract Acceleration: ", retract_acceleration );
    SERIAL_EOL;
  }
  if (code_seen('T'))
  {
    travel_acceleration = code_value();
    SERIAL_ECHOPAIR("Setting Travel Acceleration: ", travel_acceleration );
    SERIAL_EOL;
  }

}

What do you think? It's also slightly faster and it will help the transition to the new syntax...

alexborro commented 9 years ago

@alexrj the commands are sequential. With current implementation if you send: M204 Sx Py Tz The Printing Accel will be "y" and Travel Accel will be "z", I mean, the later commands will replace the former one.

If you send: M204 Sx Py The Printing Accel will be "y" and Travel Accel will be "x".

I strongly suggest using the 3 parameters: M204 Sx Py Tz And in a further version you can remove the Sx option..

We have started tagging Marlin Stable version. By now we have the 1.0.2 tag. Soon it will be 1.0.3. But this is just a way to keep track of the development. We have daily struggle to keep it stable and fix bugs as soon as possible...

By the way, Why slicers do not use G2 and G3 arc moves commands??

Cheers.

Alex.

alranel commented 9 years ago

@alexborro, but Slic3r does not know the value for travel acceleration. It would need one more config setting from users in order to supply that additional value. Changing travel acceleration during print is not much useful. I only want to alter printing acceleration…

Regarding G2 and G3, Slic3r supported G2 and G3 a long time ago, then that code was not maintained anymore. Maybe one day I'll update it, but there's not much interest for it.

alexborro commented 9 years ago

Ok @alexrj , no problem providing M204 Sx Py for now. But I suggest implementing the Travel Accel option, it makes a huge difference on print time. I usually use lower Accel for printing (500~1000) and higher for traveling (3000).. Now you have separate Accel for Infill, Perimeters, Bridges and so one. You can add one more for traveling.. I think it is a nice improvement and quite easy to implement.

About G2/G3 I have checked in Marlin and it is working fine as far as I could test. As advantage we have:

If you wanna give it a try, I can provide all the firmware support you need.

Cheers.

Alex.

thiagopeixoto16 commented 9 years ago

By the way, I think the "default" acceleration field controls the acceleration for travel movements too: image

Please let me know if I'm wrong.

alexborro commented 9 years ago

@thiagopeixoto16 maybe the Default is the Travel Accel... I will let this to @alexrj answer.

alranel commented 9 years ago

Hm, using Default Acceleration as the value for T could be an idea. At the moment it's used for resetting acceleration after perimeters, after infill, after bridges and after first layer. This allows the user to only provide custom acceleration values for some of those (for example, only for perimeters). Changing the semantics of Default Acceleration so that it also applies to travel moves might work...

alranel commented 9 years ago

Wait, if I supply Tx then old Marlins will use it for retraction. That's not good!

wolfmanjm commented 9 years ago

I would like to point out Marlin is not the only firmware. It is unlikely that Smoothie, for instance, will implement this change in acceleration syntax any time soon. Maintaining backwards compatibility should be a priority for all firmware developers. Changes like this are unnecessary, you can either create a new Mxxx code or say use M204.1, as subcodes are part of the gcode spec. (although not currently supported in smoothie)

alexborro commented 9 years ago

You can use S and P instead of T. Just be sure the P takes place after S (M204 S3000 P800 = M204 P800 T3000).

We will start tagging Marlin by date, so you can create a new firmware flavor.

Unfortunately GCode standard is meant for milling machines and do no provide enough support for 3D Printing... So we need to create new ones and adapt the meaning of old ones.

Cheers.

Alex. Em 18/03/2015 20:00, "Alessandro Ranellucci" notifications@github.com escreveu:

Wait, if I supply Tx then old Marlins will use it for retraction. That's not good!

— Reply to this email directly or view it on GitHub https://github.com/alexrj/Slic3r/issues/2745#issuecomment-83218737.

alranel commented 9 years ago

@alexborro, okay, I'll create a distinct "Marlin" G-code flavor and use M204 S3000 P800.

I actually agree with @wolfmanjm; this syntax change was not necessary but I'm glad we found a retrocompatible way for keeping a single "Marlin" G-code flavor in Slic3r. Supporting multiple Marlin G-code flavors would be a pain (also for users), and it would be very awkward if we would had to use date tags for naming those G-code flavors (i.e. "Marlin before Mar11-2015", "Marlin after Mar11-2015"…).

alexborro commented 9 years ago

I usually think on backwards compatibility, but in this case the used letters had no meaning at all.. Em 18/03/2015 21:16, "Alessandro Ranellucci" notifications@github.com escreveu:

@alexborro https://github.com/alexborro, okay, I'll create a distinct "Marlin" G-code flavor and use M204 S3000 P800.

I actually agree with @wolfmanjm https://github.com/wolfmanjm; this syntax change was not necessary but I'm glad we found a retrocompatible way for keeping a single "Marlin" G-code flavor in Slic3r. Supporting multiple Marlin G-code flavors would be a pain (also for users), and it would be very awkward if we would have to use date tags for naming those G-code flavors (i.e. "Marlin before Mar11-2015", "Marlin after Mar11-2015"…).

— Reply to this email directly or view it on GitHub https://github.com/alexrj/Slic3r/issues/2745#issuecomment-83231857.

lordofhyphens commented 9 years ago

A subcode to specialize the travel acceleration would still have been less damaging to the ecosystem (as Marlin certainly is not the only firmware in town).

SystemsGuy commented 9 years ago

alexborro you mainly use a printer than has a single tool? So no need to support toolchanges?

I also think it's very short sighted to "adapt the meaning of old ones" when that means "break the old functionality".

thiagopeixoto16 commented 8 years ago

I think that was workarounded so I'm closing. Please let me know if it is not and we can reopen :)