cp2004 / OctoPrint-EEPROM-Marlin

A plugin for editing the EEPROM data of Marlin Firmware, from within OctoPrint.
https://plugins.octoprint.org/plugins/eeprom_marlin
GNU Affero General Public License v3.0
34 stars 14 forks source link

Release Candidate 3.0.0rc3 & rc4 Feedback #16

Closed cp2004 closed 3 years ago

cp2004 commented 3 years ago

Marlin EEPROM Editor 3.0.0rc3 & rc4 Feedback

Installation

Install using the following URL in OctoPrint's plugin manager > get more > ...from URL:

https://github.com/cp2004/OctoPrint-EEPROM-Marlin/archive/3.0.0rc4.zip

Once that is installed (and in OP 1.5.0+) you will be able to select 'release candidate' as the release channel (under 'Software Update'), to get future RC updates automatically: image

For a full overview of the features that have gone into this release, please read the release notes

Known Issues

Reporting an issue

Please open a full bug report in the repository, providing full OctoPrint logs. Feel free to comment on this post to provide feedback, but for bugs please open a full report. Thank you!

Scheduled release timeline

DodgeDeBoulet commented 3 years ago

The URL appears to be invalid, Charlie. Plugin Manager hangs, and when I attempt to open the link from my browser I get a 404.

cp2004 commented 3 years ago

@DodgeDeBoulet sorry, it was missing the c of rc! Fixed

cp2004 commented 3 years ago

As a follow up, https://github.com/OctoPrint/OctoPrint/issues/3886

DodgeDeBoulet commented 3 years ago

So far so good! Vastly improved UI ... much better organization. Backup works too! Great job!

DodgeDeBoulet commented 3 years ago

Actually ... didn't notice this at first. Seems to be some sort of formatting issue: image

cp2004 commented 3 years ago

We're on it, seems something in the recent Marlin broke the standard with parsing - it should be key:value, but then someone put the time in there, which breaks this. Not something I will fix my end, since it gets the data straight out of OctoPrint - but it will be fixed in OctoPrint.

Discussion (and resolution): https://discord.com/channels/704958479194128507/705047010641838211/788136463732047932

DodgeDeBoulet commented 3 years ago

So it's splitting on the colon character, I guess? Interesting that it doesn't choke on the Source Code URL ...

KenLucke commented 3 years ago

One item missing in the listing is the Filament Runout Sensor status (enabled/disabled)

DodgeDeBoulet commented 3 years ago

It would also nice to be able to get and set the TMC motor current (M903). It's stored in EEPROM but not displayed with M503.

cp2004 commented 3 years ago

One item missing in the listing is the Filament Runout Sensor status (enabled/disabled)

@KenLucke Next release, can be quite easily done - I designed it to be easily extendable with new entries. Initial release includes everything that was in the old plugin plus a couple of new ones, more can be added. This one needs to get out the door as soon as possible, and since it is in RC phase I won't be adjusting much now. Hopefully the development cycle can be quicker too. Thanks for the sponsorship BTW, much appreciated!

It would also nice to be able to get and set the TMC motor current (M903). It's stored in EEPROM but not displayed with M503.

@DodgeDeBoulet I don't think I'll be able to do this without the printer telling me first that it supports TMC motor current adjustment. If you open a request upstream in Marlin to get it reported on an M503 then I will be more than happy to listen for it, but without it I won't be able to show a field to edit if I don't even know if it exists or not.

DodgeDeBoulet commented 3 years ago

Please bear with me here, as I'm going by how I think things work, without having any real insight "under the covers."

You populate the fields in the EEPROM editor by sending an M503, right? And the printer's capabilities are interrogated by OctoPrint upon connection, so you're probably getting the values for the Firmware Info page from some OctoPrint API rather than sending an M115. I assume that's the case since I'm not seeing an M115 in the serial output when I click the LOAD button.

With regard to the settings M503 doesn't display, though: In my experience so far, Marlin seems to display the active values for commands that change settings when sent without arguments, but those settings aren't necessarily the stored EEPROM values. I do think most of them are stored to EEPROM on an M500, though. Let's call these "dynamic values."

Perhaps you could have a category for "dynamic settings" that would be populated by sending some subset of GCode commands (like M903, for TMC Current) that would allow the user to view those dynamic values and optionally set them temporarily (without an M500) or permanently (with M500).

I can also envision, sometime in the distant future, a feature where the user could add custom pages to the EEPROM editor by supplying a page name with a series of GCode commands and their associated parameters, values, data types and optional units (mm/steps/degrees/mA/etc.) for these "dynamic values."

You don't have anything better to do, right? 😂

cp2004 commented 3 years ago

You are correct about how both the EEPROM fields and firmware info are populated, the plugin sends an M503, listens for the response and anything it recognises from the output is used. OctoPrint has an internal hook for the FW info.

'Dynamic values' page could work with a set of buttons that would retrieve the values manually, I can see that happening. The way the UI framework was constructed would probably allow for your second suggestion too, since this is pretty much how I do it (there is a JS mapping of all the things, then the UI is constructed 'dynamically) already.

I would have to go and interrogate the Marlin guys (or the source code) about how the commands work, since I don't have a machine that does all the things itself - I added a number of them to the Virtual Printer in OctoPrint so I could test this plugin, and I can do this again but it requires kind of 'reverse engineering' Marlin (if you can call it that, given it is open source) so does take a bit of time.

Thanks for the great feedback though, while it is not something for the initial V3 release I have left lots of room for improvement that the previous iteration of this plugin didn't really have. 🙂

DodgeDeBoulet commented 3 years ago

Another thing that just occurred to me ... You know that M503 doesn't actually show the settings stored in EEPROM, right? It displays the active settings in SRAM, which may have been updated since the last M500 was issued. I'm not sure this is a huge problem, but it does mean that EEPROM values are being changed on SAVE that may not have been intended. I'm thinking a feature request for EEPROM interrogation may be in order 😉

Apologies if I'm stating the obvious ...

cp2004 commented 3 years ago

I tried to mitigate this by sending M500 on the 'save' button, and therefore if you are hitting 'save' for it to calculate & send the necessary commands to the printer it also saves them. I couldn't think of a way round this before, and if you want to try out settings and then restore the old ones later, well, that is what the backup feature is for.

Yes it has potential to cause an issue (if slicers use any of the commands, then it will save them to EEPROM permanently too) but if it does, head to the settings and uncheck Use M503 and it will fall back to M501 which has the unintended side effect of opening the settings from EEPROM, which would instead overwrite the previous changes. I decided M503 was likely to make less of an impact, than potentially wiping out settings changed in the terminal by hand (eg, ones that the editor doesn't fiddle with) and instead only 'viewing' what we have.

I believe (not saying I'm an expert by any means, just from experience) that sending the commands one by one to query the result would show the same result?

KenLucke commented 3 years ago

@KenLucke Next release, can be quite easily done - I designed it to be easily extendable with new entries. Initial release includes everything that was in the old plugin plus a couple of new ones, more can be added. This one needs to get out the door as soon as possible, and since it is in RC phase I won't be adjusting much now. Hopefully the development cycle can be quicker too.

Oh, I have no problems with that, just noting it for the future. Currently, it's way beyond what I thought it might be.

DodgeDeBoulet commented 3 years ago

Yes, sending the commands individually shows the same result ... the SRAM value rather than the EEPROM value.

The only workflow I can see to edit just the stored EEPROM values for the settings shown by M503 is to interrogate everything with individual commands, store the results temporarily, send M502, then M503 and display for editing. On SAVE, store with M500, then replay the settings collected from the individual commands EXCLUDING the values updated previously in the editor.

Ewwww.

Marlin needs a "dump EEPROM to serial" command. Even if it's block by block in ASCII hex values it could be decoded, parsed and displayed ...

cp2004 commented 3 years ago

3.0.0rc4 has been released, with some very minor fixes/adjustments - this ticket will remain open for discussion of both.

KenLucke commented 3 years ago

I see you added the filament runout sensor status. Thank you.

cp2004 commented 3 years ago

Did I? I don't think I did, maybe it was already there. I'm confused now, let me look at what it is supposed to pick up

cp2004 commented 3 years ago

Filament diameter is there, sensor status shouldn't be.

KenLucke commented 3 years ago

CR-10 v2 (Yellow) LAN  OctoPrint  2020-12-18 10-44-49

cp2004 commented 3 years ago

Ah, got it. That's the M115 response to say that a runout sensor exists, making whether it is enabled/disabled editable will come later.

KenLucke commented 3 years ago

Ah, got it. That's the M115 response to say that a runout sensor exists, making whether it is enabled/disabled editable will come later.

Ah. OK. ICYDK, M412 [S1|0] enables/disables/reads, BTW, if no other means exist.

Send: M412 Recv: echo:Filament runout ON Recv: Filament runout distance (mm): 5.00

cp2004 commented 3 years ago

3.0.0 has been released 🎉

I'll be looking into updates for this in the new year, thank you to everyone who reported back on this release candidate - it makes me far more confident in rolling the update out when others have tested it.

Some stats on the RCs, available under https://cp2004.github.io/OctoPluginStats : image

KenLucke commented 3 years ago

LOL, somehow I timed a restart of OctoPrint 6 minutes after your release, so caught it faster than I've ever caught a new release before :)