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.

Sign up for GitHub

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

Jump to bottom

Fixed Time Motion: Fix switching on or off FTM when printing #27335

Merged
merged 9 commits into from
Aug 17, 2024
Merged

Fixed Time Motion: Fix switching on or off FTM when printing #27335

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
258ccec
M493 S0 or S1 are forbidden during a print
narno2202 Aug 7, 2024
80816de
M493 S0 ans S1 possible when printing
narno2202 Aug 8, 2024
5e30734
Merge branch 'bugfix-2.1.x' into FTM_On_Off_fix
narno2202 Aug 13, 2024
7ab808c
Merge branch 'MarlinFirmware:bugfix-2.1.x' into FTM_On_Off_fix
narno2202 Aug 16, 2024
31d4c7b
Merge branch 'bugfix-2.1.x' into FTM_On_Off_fix
narno2202 Aug 16, 2024
7aa73d7
told em so
thinkyhead Aug 17, 2024
eaa9ed4
format
thinkyhead Aug 17, 2024
7ae5980
just report
thinkyhead Aug 17, 2024
6995edd
might as well…
thinkyhead Aug 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 7 additions & 21 deletions Marlin/src/gcode/feature/ft_motion/M493.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,27 +190,20 @@ void GcodeSuite::M493_report(const bool forReplay/*=true*/) {
* R 0.00 Set the vibration tolerance for the Y axis
*/
void GcodeSuite::M493() {
struct { bool update:1, reset_ft:1, report_h:1; } flag = { false };
struct { bool update:1, report_h:1; } flag = { false };

if (!parser.seen_any())
flag.report_h = true;

// Parse 'S' mode parameter.
if (parser.seen('S')) {
const bool active = parser.value_bool();

if (active != ftMotion.cfg.active) {
switch (active) {
case false:
flag.reset_ft = true;
// fall-thru
case true:
const bool active = parser.value_bool();
if (active != ftMotion.cfg.active) {
flag.report_h = true;
stepper.ftMotion_syncPosition();
ftMotion.cfg.active = active;
break;
}
}
}
}

#if HAS_X_AXIS
auto set_shaper = [&](const AxisEnum axis, const char c) {
Expand Down Expand Up @@ -309,7 +302,7 @@ void GcodeSuite::M493() {
// TODO: Frequency minimum is dependent on the shaper used; the above check isn't always correct.
if (WITHIN(val, FTM_MIN_SHAPE_FREQ, (FTM_FS) / 2)) {
ftMotion.cfg.baseFreq.x = val;
flag.update = flag.reset_ft = flag.report_h = true;
flag.update = flag.report_h = true;
}
else // Frequency out of range.
SERIAL_ECHOLNPGM("Invalid [", C('A'), "] frequency value.");
Expand Down Expand Up @@ -370,7 +363,7 @@ void GcodeSuite::M493() {
const float val = parser.value_float();
if (WITHIN(val, FTM_MIN_SHAPE_FREQ, (FTM_FS) / 2)) {
ftMotion.cfg.baseFreq.y = val;
flag.update = flag.reset_ft = flag.report_h = true;
flag.update = flag.report_h = true;
}
else // Frequency out of range.
SERIAL_ECHOLNPGM("Invalid frequency [", C('B'), "] value.");
Expand Down Expand Up @@ -423,15 +416,8 @@ void GcodeSuite::M493() {

#endif // HAS_Y_AXIS

planner.synchronize();

if (flag.update) ftMotion.update_shaping_params();
Copy link
Member

Choose a reason for hiding this comment

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

Seems like a smart idea to synchronize before modifying the parameters that ftMotion uses to do its motion. Are we certain it is / will always be safe to call update_shaping_params while a move is still ongoing?


if (flag.reset_ft) {
stepper.ftMotion_syncPosition();
ftMotion.reset();
}
Copy link
Member

Choose a reason for hiding this comment

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

Are we certain that changing any of the above parameters does/will not require ftMotion.reset() to recalculate something/anything based on the updated parameter(s)?


if (flag.report_h) say_shaping();
}

Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/module/stepper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3422,7 +3422,7 @@ void Stepper::set_axis_position(const AxisEnum a, const int32_t &v) {
#if ENABLED(FT_MOTION)

void Stepper::ftMotion_syncPosition() {
//planner.synchronize(); planner already synchronized in M493
planner.synchronize(); // Must be called here otherwise motion freeze

// Update stepper positions from the planner
AVR_ATOMIC_SECTION_START();
Expand Down