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.

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

Changed settings are not stored to EEPROM when saved (STM32F401) #125

Closed
theSt33v opened this issue Apr 19, 2022 · 42 comments
Closed

Changed settings are not stored to EEPROM when saved (STM32F401) #125

theSt33v opened this issue Apr 19, 2022 · 42 comments
Labels
bug: Confirmed Something isn't working, the bug was confirmed.

Comments

@theSt33v
Copy link

Describe the bug
Changed settings are not stored to EEPROM when saved.

To Reproduce
Steps to reproduce the behavior:

  1. Change any setting
  2. Select "Store Settings" or send gcode M500 through terminal.
  3. Power cycle printer
  4. All settings have reverted to firmware defaults.

Expected behavior
Changed settings should survive a power cycle after being stored.

Version (please complete the following information):
Experimental build 2.1.0.1 15Apr2022 (STM32F401).

@mriscoc
Copy link
Owner

mriscoc commented Apr 19, 2022

Thank you for your report

@mriscoc
Copy link
Owner

mriscoc commented Apr 20, 2022

Please try the latest version: #128

@mriscoc mriscoc closed this as completed Apr 20, 2022
@mriscoc mriscoc reopened this Apr 20, 2022
@bullitt186
Copy link

Unfortunately, the Issue is still unresolved with Experimental-Ender3S1-STM32F4-20220420.

@mriscoc
Copy link
Owner

mriscoc commented Apr 20, 2022

ok, thank you, I'll look deeper later.

@rockstarbill
Copy link

Just confirming, the issue persists in the latest build as well.

@airs0urce
Copy link

Yes, same for me, after I restart Ender 3 S1 all settings reverted to defaults

@wamsuperman
Copy link

I am having an issue it is storing the settings but not actually using them since newest firmware I like the toolbar but it is costing me on not being able to up my acceleration

@theSt33v
Copy link
Author

I also cannot store settings with the latest build. I read on the Facebook group today that someone with an F4 board does not have this issue. It's so strange that the behavior is so inconsistent with boards that have identical hardware. Maybe the path to the EEPROM is different depending on the board, which is why the save is unsuccessful in some cases?

@theSt33v
Copy link
Author

I am having an issue it is storing the settings but not actually using them since newest firmware I like the toolbar but it is costing me on not being able to up my acceleration

Are you sure that your slicer is not defining the acceleration parameters?

@jeeltuss
Copy link

jeeltuss commented Apr 25, 2022

Hello, just want to post this hoping it can help to shed light on the issue.
I was fiddling with the experimental firmware & octoprint, while I was testing the missing store of settings I used the "Reboot printer" function in Control section.
After power cycling the printer reported "M112 Printer Killed", and asked to be manually shut down.

Then the web interface of octoprint reported this:

Your printer's firmware reported an error. Due to that OctoPrint will disconnect. Reported error: EEPROM CRC mismatch - (stored) 34127 != 59449 (calculated)!

@thesnarkyone
Copy link

I am experiencing the same issue as well. Power cycles remove settings.

@mriscoc
Copy link
Owner

mriscoc commented Apr 25, 2022

Hello, just want to post this hoping it can help to shed light on the issue. I was fiddling with the experimental firmware & octoprint, while I was testing the missing store of settings I used the "Reboot printer" function in Control section. After power cycling the printer reported "M112 Printer Killed", and asked to be manually shut down.

Then the web interface of octoprint reported this:

Your printer's firmware reported an error. Due to that OctoPrint will disconnect. Reported error: EEPROM CRC mismatch - (stored) 34127 != 59449 (calculated)!

Check error handling in https://github.com/mriscoc/Ender3V2S1/wiki/Octoprint

@jeeltuss
Copy link

Hello, just want to post this hoping it can help to shed light on the issue. I was fiddling with the experimental firmware & octoprint, while I was testing the missing store of settings I used the "Reboot printer" function in Control section. After power cycling the printer reported "M112 Printer Killed", and asked to be manually shut down.
Then the web interface of octoprint reported this:
Your printer's firmware reported an error. Due to that OctoPrint will disconnect. Reported error: EEPROM CRC mismatch - (stored) 34127 != 59449 (calculated)!

Check error handling in https://github.com/mriscoc/Ender3V2S1/wiki/Octoprint

Oh ok, I was hoping that the CRC mismatch error can give a clue on why the settings are not saved. My bad. Thanks for the link.

@mriscoc mriscoc added the bug: Confirmed Something isn't working, the bug was confirmed. label Apr 26, 2022
@theSt33v
Copy link
Author

The issue still exists in the 27Apr build.

@vb138
Copy link

vb138 commented Apr 28, 2022

Confirmed, I have the same problem.

@bankm1
Copy link

bankm1 commented May 3, 2022

+1 settings will not save

@airs0urce
Copy link

airs0urce commented May 3, 2022

As I have access to physical machine just want to let you know if you need some details to solve this issue (photos of chips etc) I can make and send them

@mriscoc
Copy link
Owner

mriscoc commented May 3, 2022

As I have access to physical machine just want to let you know if you need some details to solve this issue (photos of chips etc) I can make and send them

Thank you, I do dozens of fast tests before the release of a version. But right now, it is possible to write a G-code with all configuration parameters that can be loaded at startup.

@fabiomaticus
Copy link

The issue still exists in the 4 May build.

@mriscoc
Copy link
Owner

mriscoc commented May 5, 2022

The issue still exists in the 4 May build.

Thanks for the feedback

@jeeltuss
Copy link

jeeltuss commented May 7, 2022

For interest, please visit https://www.th3dstudio.com/hc/downloads/unified-2-firmware/creality/creality-boards-creality/creality-ender-3-s1-pro-firmware-v24s1_301-board/ and https://github.com/th3dstudio/UnifiedFirmware/commits/2.0.x

I am currently running the TH3D Unified fimware on my S1 (non Pro) with the STM32F401 chip, and I can confirm that all settings I tried are correctly saved when power-cycling the printer. Maybe this can help @mriscoc in investigating the issue.

@theSt33v
Copy link
Author

theSt33v commented May 7, 2022

I did a little bit of investigation and I think I need to clarify the issue. The settings are, in fact, stored correctly. If you save settings to EEPROM and then restore default settings, you can easily restore your previously-saved settings. This may indicate that the issue is not that the settings are not saved, but rather they are being wiped at boot.

@mriscoc
Copy link
Owner

mriscoc commented May 7, 2022

I did a little bit of investigation and I think I need to clarify the issue. The settings are, in fact, stored correctly. If you save settings to EEPROM and then restore default settings, you can easily restore your previously-saved settings. This may indicate that the issue is not that the settings are not saved, but rather they are being wiped at boot.

Yes, the issue is with the calculus of the CRC EEPROM value in the F4 chips. TH3D sources are based on Marlin 2.0.9.3, the Professional sources are based on Marlin BugFix 2.1.0.0 that is why the differences.

@mriscoc
Copy link
Owner

mriscoc commented May 7, 2022

If someone is confident with coding and compiling the firmware and has the F4 board, try to do tests with DEBUG_EEPROM_READWRITE enabled, check these relevant parts of the source:

bool PersistentStore::read_data(int &pos, uint8_t *value, size_t size, uint16_t *crc, const bool writing/*=true*/) {
do {
uint8_t * const p = (uint8_t * const)pos;
uint8_t c = eeprom_read_byte(p);
if (writing) *value = c;
crc16(crc, &c, 1);
pos++;
value++;
} while (--size);
return false;
}

void crc16(uint16_t *crc, const void * const data, uint16_t cnt) {
uint8_t *ptr = (uint8_t *)data;
while (cnt--) {
*crc = (uint16_t)(*crc ^ (uint16_t)(((uint16_t)*ptr++) << 8));
for (uint8_t i = 0; i < 8; i++)
*crc = (uint16_t)((*crc & 0x8000) ? ((uint16_t)(*crc << 1) ^ 0x1021) : (*crc << 1));
}
}

uint16_t stored_crc;
EEPROM_READ_ALWAYS(stored_crc);

and

else if (working_crc != stored_crc) {
eeprom_error = true;
DEBUG_ERROR_MSG("EEPROM CRC mismatch - (stored) ", stored_crc, " != ", working_crc, " (calculated)!");
TERN_(DWIN_LCD_PROUI, LCD_MESSAGE(MSG_ERR_EEPROM_CRC));
TERN_(HOST_EEPROM_CHITCHAT, hostui.notify(GET_TEXT_F(MSG_ERR_EEPROM_CRC)));
IF_DISABLED(EEPROM_AUTO_INIT, ui.eeprom_alert_crc());
}

@theSt33v
Copy link
Author

theSt33v commented May 7, 2022

There's not enough flash space to enable that for the F4 without cutting other things. I'm trying to disable some features to make room but I'm running into compile errors because the features are "required for proui." I'll keep at it, but if you know any features I can cut without breaking things too much, that would be helpful.

@mriscoc
Copy link
Owner

mriscoc commented May 7, 2022

Comment out in dwin_defines.h the line #define ProUIex 1
That will give you flash space.

@theSt33v
Copy link
Author

theSt33v commented May 7, 2022

Doing that caused build errors, but I was able to get a successful and functional build by disabling M503 in configuration.h, setting HAS_GCODE_PREVIEW to 0 in proui.h and commenting out the error if for HAS_GCODE_PREVIEW in dwin_defines.h. I'll see what I can find out with this CRC problem.

Could you give me some more background on what is known so far about the problem? How do we know that the issue is CRC calculation related? So far, I haven't been able to trigger any of the CRC errors.

@jeeltuss
Copy link

jeeltuss commented May 7, 2022

Doing that caused build errors, but I was able to get a successful and functional build by disabling M503 in configuration.h, setting HAS_GCODE_PREVIEW to 0 in proui.h and commenting out the error if for HAS_GCODE_PREVIEW in dwin_defines.h. I'll see what I can find out with this CRC problem.

Could you give me some more background on what is known so far about the problem? How do we know that the issue is CRC calculation related? So far, I haven't been able to trigger any of the CRC errors.

As I was saying here, I had octoprint reporting errors from the printer regarding crc mismatch.
I remember saving settings straight from the printer after modifying any of them (i.e. the z offset), while octoprint was connected. Hope this helps.

@mriscoc
Copy link
Owner

mriscoc commented May 8, 2022

As I was saying here, I had octoprint reporting errors from the printer regarding crc mismatch. I remember saving settings straight from the printer after modifying any of them (i.e. the z offset), while octoprint was connected. Hope this helps.

Yes, you were in the correct way.

@mriscoc
Copy link
Owner

mriscoc commented May 8, 2022

Of course, it is possible to disable the crc checking but it could be dangerous, but maybe it is a simple test to check if only the crc is the problem.

else if (working_crc != stored_crc) {

else if (0) {

@theSt33v
Copy link
Author

theSt33v commented May 8, 2022

I just did that and it is indeed the problem. If you change eeprom_error=true to false, settings are preserved after power cycle.

@mriscoc
Copy link
Owner

mriscoc commented May 8, 2022

So, now we need to understand why only for the F4 is working_crc != stored_crc

@theSt33v
Copy link
Author

theSt33v commented May 8, 2022

Right. Your code in that section is the same as Creality source, and stock firmware does not have this issue, so it's very strange.

@mriscoc
Copy link
Owner

mriscoc commented May 8, 2022

Creality source is based on STM32 Maple I think, that uses a different HAL implementation. The Marlin bugfix branch has deprecated the Maple version and uses STM libraries.

@theSt33v
Copy link
Author

theSt33v commented May 9, 2022

I did a lot of poking around and I only ever get that CRC error at boot. All other commands that trigger CRC checks (M500 and M501) are always successful and return CRC values that are consistent and change when they're supposed to. Even when I disable the automatic factory reset (M502) that the error usually triggers, the CRC checks pass. This means that even when you do literally nothing to the flash after the apparent corruption detected by the first check, the CRC checks pass after boot.

It seems to me that only the CRC check that happens at boot is problematic, and it's a false positive. Is there a different section of code that governs the CRC check at boot? Are the working_crc and/or stored_crc values incorrectly assigned at boot due to an incorrect hardcoded value somewhere in the settings, but correctly assigned when triggered post-boot? I don't know, but I'm going to just disable the error in the builds that I use. I'd like to get to the root of this, but I think I've reached the limit of my very limited ability to diagnose this stuff. All I can say is all the relevant sections of code that were posted earlier are nearly identical to their equivalent sections in the TH3D unified firmware (which doesn't suffer from this issue), so I don't think the problem is there.

@mriscoc
Copy link
Owner

mriscoc commented May 10, 2022

But why does that error only occur for the F4 SoC? Unfortunately, I don't have an F4 board to do debugging.
From your tests I can summarize:
M500 Save data to EEPROM and store a correct calculated (working) CRC
M501 Read data from EEPROM and the stored CRC is equal to calculated (working) CRC
M997 (Reboot) triggers the error working_crc != stored_crc, but what CRC is bad? the working, the stored, or both?

@mriscoc mriscoc closed this as completed May 10, 2022
@mriscoc mriscoc reopened this May 10, 2022
@theSt33v
Copy link
Author

theSt33v commented May 10, 2022

That's correct. As for which one is bad, it wasn't consistent in my testing. One of them always matched the CRC from before the reboot (which makes it the correct one), but sometimes it was working and sometimes it was stored. I'll do more systematic tests today to see if there is a pattern.

UPDATE: In all of my testing so far, stored_crc is the correct value. I thought I saw cases yesterday where working_crc was correct, but I cannot replicate that today. I think "why only F4" is a good question to ask. That means the problem may be with code that is F4-specific and could have an effect on CRC calculations. I think another helpful question would be what action is happening at boot that results in a CRC check? Is it a read or write operation?

@mriscoc
Copy link
Owner

mriscoc commented May 16, 2022

Fixed in the last release candidate version #128 (comment)

@vb138
Copy link

vb138 commented May 16, 2022

Tested. Now all settings are preserved even after rebooting the device. Thank you.

@mriscoc
Copy link
Owner

mriscoc commented May 16, 2022

Tested. Now all settings are preserved even after rebooting the device. Thank you.

Thank you for your confirmation. I'm closing the issue now.

@mriscoc mriscoc closed this as completed May 16, 2022
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug: Confirmed Something isn't working, the bug was confirmed.
Projects
None yet
Development

No branches or pull requests