amsbr / OctoPrint-EEPROM-Marlin

OctoPrint-EEPROM-Marlin
15 stars 31 forks source link

Plugin restoring stored EEPROM settings with M501 when OctoPrint settings are viewed #30

Open tophattwaffle opened 6 years ago

tophattwaffle commented 6 years ago

When viewing the settings for OctoPrint, the EEPROM editor plugin sends M501 to the printer. This will restore the stored EEPROM to be live. However this is not ideal as some settings may be set on a per-print basis.

Example: Automatic Bed Leveling only after G29. After a G29, Marlin will set M420 S1, enabling bed leveling. In my EEPROM I have M420 S0 set so ABL is only enabled after a G29 has generated the mesh. Opening my OctoPrint settings results in M501 being sent to my printer, setting M420 S0, disabling ABL - Along with whatever other print specific settings I may have had set to override EEPROM values.

I don't think the plugin should be restoring settings just for viewing it's settings. I'm guessing you've used M501 to get the settings into terminal so you can parse them out for the view the stored settings. If there was some sort of warning to let people know loading settings may cause issues, that would be nice. Or maybe a toggle to switch between M501 (stored) and M503 (live) settings.

This was a fun few weeks of troubleshooting to where I have now removed this plugin as it has been killing my prints.

Desterly commented 6 years ago

The problem I see with this is there is no current command in Marlin to "View" EEPROM Data only:

M500 - Save to EEPROM M501 - Load from EEPROM M502 - Reset EEPROM M503 - View Memory

M501 is no different than turning the printer off and back on.

Are you saying there are times that you are changing printer settings but not saving them to EEPROM for multiple jobs?

I'm also not sure how this affects ABL overall. ABL is disabled every G28 and is only enabled with a G29 and/or M420 S1

M420 S can be used to enable/disable bed leveling. 
For example, M420 S1 must be used after M501 to enable the loaded mesh or matrix, 
and to re-enable leveling after G28, which disables leveling compensation.

What you are looking for is a M505 - View EEPROM data. This doesn't exist.

The other problem you have is a M500 would save settings from your M503 if not pulled in from M501. Lets say you change your Z offset from -2.1 to -2.4 for testing. If you M500 regardless of how/why, your Z offset is now -2.4 in EEPROM. If the editor didn't always pull from EEPROM to set current values, a M500 would set incorrect items.

The only alternative that I could see would be if the plugin did a M503 to bring in the settings, M501 to load, then after a "save" or "close" of the editor, restored the values from the M503.

Another possibility would be that the edit showed the results of a M503 AND M501. If there is a difference there, the "Upload" button is disabled preventing you from updating any values set since the last M500. This one might be possible

Or am I missing something else?

tophattwaffle commented 6 years ago

You are correct, a M505 would solve all of this. I'm going to make a request over at Marlin to see if this is possible.

It effects ABL for many users as M420 S0 is set in the eeprom. The starting script then contains a G28 (home and clear old mesh) followed by G29 which makes a new mesh and saves it.

The issue is that if you're in the middle of a print and ABL is on, this plug-in does a M501 and it loads the M420 S0 that ends up disabling ABL.

On Tue, Aug 21, 2018, 16:01 Desterly notifications@github.com wrote:

The problem I see with this is there is no current command in Marlin to "View" EEPROM Data only:

M500 - Save to EEPROM M501 - Load from EEPROM M502 - Reset EEPROM M503 - View Memory

M501 is no different than turning the printer off and back on.

Are you saying there are times that you are changing printer settings but not saving them to EEPROM for multiple jobs?

I'm also not sure how this affects ABL overall. ABL is disabled every G28 and is only enabled with a G29 and/or M420 S1

M420 S can be used to enable/disable bed leveling. For example, M420 S1 must be used after M501 to enable the loaded mesh or matrix, and to re-enable leveling after G28, which disables leveling compensation.

What we ultimately need though is a M505 - View EEPROM data. The only alternative that I could see would be if the plugin did a M503 to bring in the settings, M501 to load, then after a "save" or "close" of the editor, restored the values from the M503.

Or am I missing something else?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/amsbr/OctoPrint-EEPROM-Marlin/issues/30#issuecomment-414820612, or mute the thread https://github.com/notifications/unsubscribe-auth/AGddbbc3_ABc6kdsGB01RLBzYRo22wDAks5uTHVHgaJpZM4WGUGj .

Desterly commented 6 years ago

While I'm not sure exactly what difference enabling/disabling bed leveling (unless you are also referring to Z compensation) has during printing, based off your updated description, what you are looking for is prevention of clicking "Load" and issuing the M501 if printing is active.

This would prevent the M501 being issued during an active job. G28 always disables bed leveling which means in startup scripts, a G29 or M420 S1 MUST be issued to enable bed leveling anyway.

As a side note, G29 does not save the new mesh. unless I'm drastically mistaken, only a M500 will actually store the new mesh to eeprom.

Ideally though, I think a M500-M504 should all be prevented/disabled during an active job

tophattwaffle commented 6 years ago

Yes, bed leveling as in z compensation.

And yes - if a print is active a M501 should never be issued. Thought preventing M500-504 during active print also works.

With EZABL a G29 is issued every print and the mesh is never saved.

On Tue, Aug 21, 2018, 20:30 Desterly notifications@github.com wrote:

While I'm not sure exactly what difference enabling/disabling bed leveling (unless you are also referring to Z compensation) has during printing, based off your updated description, what you are looking for is prevention of clicking "Load" and issuing the M501 if printing is active.

This would prevent the M501 being issued during an active job. G28 always disables bed leveling which means in startup scripts, a G29 or M420 S1 MUST be issued to enable bed leveling anyway.

As a side note, G29 does not save the new mesh. unless I'm drastically mistaken, only a M500 will actually store the new mesh to eeprom.

Ideally though, I think a M500-M504 should all be prevented/disabled during an active job

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/amsbr/OctoPrint-EEPROM-Marlin/issues/30#issuecomment-414876442, or mute the thread https://github.com/notifications/unsubscribe-auth/AGddbbI5ejxGx4J4cL9-meVGslaiyXlBks5uTLRIgaJpZM4WGUGj .

Desterly commented 6 years ago

OK overall I see the issue...

This is actually a fairly easy fix as I recall (I've looked at the plugin a few times). I'll see what I can do and issue a pull request to disable the admin functions during a print (if amsbr doesn't get to it first)

Desterly commented 6 years ago

Take a look at pull request #32 - This is a small modification that effectively disables the Add-In if the printer is currently "Printing" or "Paused"

houseofbugs commented 5 years ago

https://github.com/houseofbugs/OctoPrint-EEPROM-Marlin/archive/master.zip