M300 reset

[bmuessig]

Trying the following command will emit an approx. 4s tone (which the buzzer struggles to do) and in turn crash and reset the printer: M300 P10000 S7000

I suspect that there could either be an integer overrun or - much more likely - a WDT reset due to the too long tone. Much larger time values only yield a short tone.

[leptun]

@bmuessig Is this happening in one of the releases or in the latest Mk3 branch?

[bmuessig]

@leptun This is happening in release 3.3.0. I unfortunately cannot test any more recent version.

[bmuessig]

I guess that this has not been fixed in the meantime though.

[Panayiotis-git]

The issue exists also to the latest version

[leptun]

Found the problem. I'll test tomorrow and maybe even make a PR.

[techfreek]

Still seeing this on 3.9.1 (albeit with a 5 second beep, not a 4 second beep)

[Jaxx005]

@techfreek [edit: this line added in case not seen, as post was already closed :) ]

Latest release FW (V3.10.0) on MK3S has the problem - it seems related to the P period:

M300 P2000 S0 ; silence for 2sec as expected M300 P5000 S0 ; causes printer restart!

Pronterface test confirms this line as problematic - note the ERROR after reboot (??). Here's the Pronterface USB/serial output for M300 with P2000 and then P5000:

>>>M300 P2000 S0 SENDING:M300 P2000 S0 >>>M300 P5000 S0 SENDING:M300 P5000 S0 start echo: 3.10.0-4481 echo: Last Updated: May 6 2021 13:52:17 | Author: (none, default config) Compiled: May 6 2021 echo: Free Memory: 2054 PlannerBufferBytes: 1760 echo:Stored settings retrieved adc_init Extruder fan type: NOCTUA CrashDetect ENABLED! E-motor current scaling enabled E-motor current scaling enabled E-motor current scaling enabled E-motor current scaling enabled E-motor current scaling enabled E-motor current scaling enabled FSensor ENABLED (sensor board revision: 0.4 or newer) E-motor current scaling enabled Sending 0xFF Error:volume.init failed [ERROR] Error:volume.init failed

LCD status changed MMU not responding - DISABLED

NOTES:
Using the G4 (DWELL) instead of "M300 P12345 S0" appears to function OK. Example (tested OK):

G4 P5000 ; pause for 5sec, eg. between sounds

TTFN Cheers

[techfreek]

I kinda wonder if a "quick hacky fix" for this is just to limit the duration of the M300 to be less than the max timeout of the watchdog/whatever is detecting the hang

[bmuessig]

M300 is implemented here: https://github.com/prusa3d/Prusa-Firmware/blob/MK3/Firmware/Marlin_main.cpp#L7793

I would however recommend changing the Sound_MakeCustom method in general by modifying the delay to be executed in a loop, where the WDT is reset every iteration. This method can be found here: https://github.com/prusa3d/Prusa-Firmware/blob/MK3/Firmware/sound.cpp#L66

[bmuessig]

On second thought, it would be a wise idea to implement the duration of the beep similar to how dwell is handled:

st_synchronize(); // NOTE: I don't know if it is reasonable to keep this inside a sound method
ms += _millis();
while(_millis() < ms) {
  manage_heater();
  manage_inactivity();
  lcd_update(0); // NOTE: It would make sense to remove this for the tone
}

This would ensure that all critical tasks are still being taken care of during long beeps and it would act similar to a regular delay, but with the desired sound. Generally, it would make sense to only run this loop for sounds that exceed a certain threshold, say 500ms. I generally would not pause the regular updating of the printer for extended periods of time, so this compromise should make sure that the printer keeps operating safely. As the actual sound generation is taken care of in a timer, added delays caused by the continued updating should not interfer with the sound generation itself and they should at worst add a marginal bit of delay to a command, for which slight delay inaccuracy should not matter at all.

[Jaxx005]

That’s what I was wondering. Yes a bit ‘hacky’ but better than just a ‘crash’. As long as it’s documented too 😊

From: Alex Bahm @.> Sent: Sunday, 05 December 2021 7:00 PM To: prusa3d/Prusa-Firmware @.> Cc: Jaxx005 @.>; Comment @.> Subject: Re: [prusa3d/Prusa-Firmware] M300 reset (#2045)

I kinda wonder if a "quick hacky fix" for this is just to limit the duration of the M300 to be less than the max timeout of the watchdog/whatever is detecting the hang

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/prusa3d/Prusa-Firmware/issues/2045#issuecomment-986282278 , or unsubscribe https://github.com/notifications/unsubscribe-auth/AEVXL3F6AH3XRQQ7KHLKIJTUPOZBJANCNFSM4IG5URMA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub . https://github.com/notifications/beacon/AEVXL3H2A3ADAB5X4AQXDNTUPOZBJA5CNFSM4IG5URMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOHLEXSJQ.gif

[bmuessig]

That’s what I was wondering. Yes a bit ‘hacky’ but better than just a ‘crash’. As long as it’s documented too

For a quick fix, it would be a good idea to implement the maximum time-limiting in Sound_MakeCustom in Firmware/sound.cpp. This would cause other usages of the custom sound method to also respect the WDT. It's also a good idea to add a print statement that logs if a command used a too long print.

Generally, I'd recommend fixing it correctly by implementing the beep 'dwell-style', which should only take a couple of minutes to do and would be IMO the more suitable solution, as it ensures that the general updating is not paused during beeps, as it is currently.

[bmuessig]

On third glance, how about simply using delay_keep_alive in Sound_MakeCustom instead of _delay. I believe that is the cleanest and at the same time simplest solution that should avoid any WDT reset. Additionally, we can still limit the maximum duration of M300 to something reasonable (e.g. 10s).

[leptun]

Using delay_keep_alive would stop the crashing, but any beep from the UI while M300 is executing would stop the M300 beep, but keep waiting without any beeping.

[bmuessig]

Using delay_keep_alive would stop the crashing, but any beep from the UI while M300 is executing would stop the M300 beep, but keep waiting without any beeping.

Good point! Generally, I'd suggest to only use delay_keep_alive for sufficiently long delays (e.g. > 1s).

I see two options to avoid the issue (and a bad, third one):

1. Modify the custom sound generation method to use a modified version (without LCD updates) of delay_keep_alive for long delays
2. Introduce a guard flag that ensures that the Sound_MakeCustom or its friends can only be called once and aren't re-entrant
3. Introduce a sound effects stack (this is in my opinion an absolute overkill given the added complexity)

Code example for option 2:

uint8_t sound_active = 0;

void Sound_MakeCustom(...)
{
  if (sound_active)
    return;
  sound_active = 1;

  ... do stuff that potentially uses delay_keep_alive

  sound_active = 0;
}

[bmuessig]

Alright, I've prepared a pull request that implements my first proposed solution, which modifies Sound_MakeCustom to use a delay method that suits the delay:

For delays...

• below 1.1s, use the regular delay method
• above, use delay_keep_alive_headless, a modified version of delay_keep_alive that does not update the LCD

This should be able to take care of all WDT reset problems concerning sound.

[bmuessig]

Without LCD updates (which also take care of buttons and menus), there should be no possible collisions between sounds. What do you think, @leptun?

[github-actions[bot]]

This issue has been flagged as stale because it has been open for 60 days with no activity. The issue will be closed in 7 days unless someone removes the "stale" label or adds a comment.

[bmuessig]

Bump

[github-actions[bot]]

This issue has been flagged as stale because it has been open for 60 days with no activity. The issue will be closed in 7 days unless someone removes the "stale" label or adds a comment.

[github-actions[bot]]

Thank you for your contribution to our project. This issue has not received any updates for 60 days and may be considered "stale." If this issue is still important to you, please add an update within the next 7 days to keep it open. Administrators can manually reopen the issue if necessary.

[bmuessig]

Bump