search

MaslowCNC / Firmware

Maslow Firmware
GNU General Public License v3.0
263 stars 134 forks source link

Fix to EEPROM write issue #489

Closed madgrizzle closed 5 years ago

madgrizzle commented 5 years ago

This adds a flag (writeStepsToEEPROM) to the sys structure that is raised whenever an axis is attached. The systemSaveAxesPosition() function tests that this flag is true and that all axes are detached. If so, it calls settingsSaveStepstoEEprom() and after the EEPROM is saved, it clears the flag. Therefore, the only time steps should be saved to EEPROM is after an axis becomes attached and then when all axes become detached. So that's should only happen 2 seconds of idle after a motor move.

MaslowCommunityGardenRobot commented 5 years ago

Congratulations on the pull request @madgrizzle

Now we need to decide as a community if we want to integrate these changes. You should vote by giving this comment a thumbs up or a thumbs down. Votes are counted in 48 hours. Ties will not be merged.

I'm just a robot, but I love to see people contributing so I'm going vote thumbs up (but my vote won't count...)!

BarbourSmith commented 5 years ago

This looks very cleanly implemented. Excellent work!

I'm still pretty surprised that this issue exists. I know for a fact that we used to write to the eeprom only once two seconds after the motor was detached because I remember testing that with print statements early on, but the way the system works was refactored later and we could have changed the behavior.

I don't have access to an aduino right now to test, but can we confirm that a print statement added at line 163 in Settings.cpp EEPROM.put(310, sysSteps);, runs regularly?

If we are concerned about EEPROM fatigue we can move to a new location in memory from now on. I know we have a lot of extra slots although we would have to make it so the position was moved to the new location automatically so people won't be bothered by the change

madgrizzle commented 5 years ago

I can try it out with fakeservo tommorow/this weekend. If surprised it's writing, but I'm working on porting code to a teensy and in testing the motors, I ran into issues and the culprit was that it was writing to eeprom during the maslowDelay calls.. once I prevented it writing, my issues went away.

madgrizzle commented 5 years ago

Oh, cool.. I updated the code in patch-4 and it added it to the PR.

MaslowCommunityGardenRobot commented 5 years ago

Time is up and we're ready to merge this pull request. Great work!