mirror/Grbl_Esp32
1
0
Fork 0
mirror of https://github.com/bdring/Grbl_Esp32.git synced 2025-08-29 09:10:03 +02:00
Code Issues Releases Wiki Activity

Fixed strange WCO values on first load (#702)

Browse Source 
When loading Grbl_Esp32 into a fresh ESP32, the WCOs
would often have strange, very large, values.  The problem
was the code that tries to propagate data from the old
"Eeprom" storage format into the new NVS scheme.  The old
format had a broken checksum computation that made the
checksum so weak that if succeeds about half the time on
random data.

The solution is to get rid of all that old code.  The downside
is that migration from a build that uses the old format will
lose the WCO values.  The user will have to reestablish them.
Subsequent updates between different versions that both use
the new NVS format will propagate WCO values correctly.
This commit is contained in:
Mitch Bradley
2020-12-10 04:34:44 -10:00
committed by GitHub
parent b627f31e3b
commit 8e3e23c148
7 changed files with 36 additions and 158 deletions
Download Patch File Download Diff File Expand all files Collapse all files

81
Grbl_Esp32/src/Eeprom.cpp
View File

@@ -1,81 +0,0 @@
/*
Eeprom.cpp - Coordinate data stored in EEPROM
Part of Grbl
Copyright (c) 2014-2016 Sungeun K. Jeon for Gnea Research LLC
2018 - Bart Dring This file was modifed for use on the ESP32
CPU. Do not use this with Grbl for atMega328P
Grbl is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.
Grbl is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with Grbl. If not, see <http://www.gnu.org/licenses/>.
*/
#include "Grbl.h"
void memcpy_to_eeprom_with_checksum(unsigned int destination, const char* source, unsigned int size) {
unsigned char checksum = 0;
for (; size > 0; size--) {
unsigned char data = static_cast<unsigned char>(*source++);
// Note: This checksum calculation is broken as described below.
checksum = (checksum << 1) || (checksum >> 7);
checksum += data;
EEPROM.write(destination++, *(source++));
}
EEPROM.write(destination, checksum);
EEPROM.commit();
}
int memcpy_from_eeprom_with_old_checksum(char* destination, unsigned int source, unsigned int size) {
unsigned char data, checksum = 0;
for (; size > 0; size--) {
data = EEPROM.read(source++);
// Note: This checksum calculation is broken - the || should be just | -
// thus making the checksum very weak.
// We leave it as-is so we can read old data after a firmware upgrade.
// The new storage format uses the tagged NVS mechanism, avoiding this bug.
checksum = (checksum << 1) || (checksum >> 7);
checksum += data;
*(destination++) = data;
}
return (checksum == EEPROM.read(source));
}
int memcpy_from_eeprom_with_checksum(char* destination, unsigned int source, unsigned int size) {
unsigned char data, checksum = 0;
for (; size > 0; size--) {
data = EEPROM.read(source++);
checksum = (checksum << 1) | (checksum >> 7);
checksum += data;
*(destination++) = data;
}
return (checksum == EEPROM.read(source));
}
// Read selected coordinate data from EEPROM. Updates pointed coord_data value.
// This is now a compatibility routine that is used to propagate coordinate data
// in the old EEPROM format to the new tagged NVS format.
bool old_settings_read_coord_data(uint8_t coord_select, float* coord_data) {
uint32_t addr = coord_select * (sizeof(float) * N_AXIS + 1) + EEPROM_ADDR_PARAMETERS;
if (!(memcpy_from_eeprom_with_old_checksum((char*)coord_data, addr, sizeof(float) * N_AXIS)) &&
!(memcpy_from_eeprom_with_checksum((char*)coord_data, addr, sizeof(float) * MAX_N_AXIS))) {
// Reset with default zero vector
clear_vector_float(coord_data);
// The old code used to rewrite the zeroed data but now that is done
// elsewhere, in a different format.
return false;
}
return true;
}
// Allow iteration over CoordIndex values
CoordIndex& operator ++ (CoordIndex& i) {
i = static_cast<CoordIndex>(static_cast<uint8_t>(i) + 1);
return i;
}

61
Grbl_Esp32/src/Eeprom.h
View File

@@ -1,61 +0,0 @@
#pragma once
/*
Eeprom.h - Header for system level commands and real-time processes
Part of Grbl
Copyright (c) 2014-2016 Sungeun K. Jeon for Gnea Research LLC
2018 - Bart Dring This file was modifed for use on the ESP32
CPU. Do not use this with Grbl for atMega328P
Grbl is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.
Grbl is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with Grbl. If not, see <http://www.gnu.org/licenses/>.
*/
#include "Grbl.h"
// Define EEPROM memory address location values for saved coordinate data.
const int EEPROM_SIZE = 1024U;
const int EEPROM_ADDR_PARAMETERS = 512U;
//unsigned char eeprom_get_char(unsigned int addr);
//void eeprom_put_char(unsigned int addr, unsigned char new_value);
void memcpy_to_eeprom_with_checksum(unsigned int destination, const char* source, unsigned int size);
int memcpy_from_eeprom_with_checksum(char* destination, unsigned int source, unsigned int size);
int memcpy_from_eeprom_with_old_checksum(char* destination, unsigned int source, unsigned int size);
// Reads selected coordinate data from EEPROM
bool old_settings_read_coord_data(uint8_t coord_select, float* coord_data);
// Various places in the code access saved coordinate system data
// by a small integer index according to the values below.
enum CoordIndex : uint8_t{
Begin = 0,
G54 = Begin,
G55,
G56,
G57,
G58,
G59,
// To support 9 work coordinate systems it would be necessary to define
// the following 3 and modify GCode.cpp to support G59.1, G59.2, G59.3
// G59_1,
// G59_2,
// G59_3,
NWCSystems,
G28 = NWCSystems,
G30,
// G92_2,
// G92_3,
End,
};
// Allow iteration over CoordIndex values
CoordIndex& operator ++ (CoordIndex& i);

6
Grbl_Esp32/src/GCode.cpp
View File

@@ -24,6 +24,12 @@
#include "Grbl.h"
// Allow iteration over CoordIndex values
CoordIndex& operator++(CoordIndex& i) {
i = static_cast<CoordIndex>(static_cast<uint8_t>(i) + 1);
return i;
}
// NOTE: Max line number is defined by the g-code standard to be 99999. It seems to be an
// arbitrary value, and some GUIs may require more. So we increased it based on a max safe
// value when converting a float (7.2 digit precision)s to an integer.

26
Grbl_Esp32/src/GCode.h
View File

@@ -232,6 +232,32 @@ enum GCParserFlags {
GCParserLaserIsMotion = bit(7),
};
// Various places in the code access saved coordinate system data
// by a small integer index according to the values below.
enum CoordIndex : uint8_t{
Begin = 0,
G54 = Begin,
G55,
G56,
G57,
G58,
G59,
// To support 9 work coordinate systems it would be necessary to define
// the following 3 and modify GCode.cpp to support G59.1, G59.2, G59.3
// G59_1,
// G59_2,
// G59_3,
NWCSystems,
G28 = NWCSystems,
G30,
// G92_2,
// G92_3,
End,
};
// Allow iteration over CoordIndex values
CoordIndex& operator ++ (CoordIndex& i);
// NOTE: When this struct is zeroed, the 0 values in the above types set the system defaults.
typedef struct {
Motion motion; // {G0,G1,G2,G3,G38.2,G80}

1
Grbl_Esp32/src/Grbl.h
View File

@@ -41,7 +41,6 @@ const char* const GRBL_VERSION_BUILD = "20201207";
#include "Defaults.h"
#include "Error.h"
#include "Eeprom.h"
#include "WebUI/Authentication.h"
#include "WebUI/Commands.h"
#include "Probe.h"

1
Grbl_Esp32/src/ProcessSettings.cpp
View File

@@ -79,7 +79,6 @@ namespace WebUI {
}
void settings_init() {
EEPROM.begin(EEPROM_SIZE);
make_settings();
WebUI::make_web_settings();
make_grbl_commands();

18
Grbl_Esp32/src/SettingsDefinitions.cpp
View File

@@ -196,9 +196,9 @@ static bool checkSpindleChange(char* val) {
}
grbl_msg_sendf(CLIENT_ALL, MsgLevel::Info, "Spindle turned off with setting change");
}
gc_state.spindle_speed = 0; // Set S value to 0
spindle->deinit(); // old spindle
Spindles::Spindle::select(); // get new spindle
gc_state.spindle_speed = 0; // Set S value to 0
spindle->deinit(); // old spindle
Spindles::Spindle::select(); // get new spindle
return true;
}
return true;
@@ -219,17 +219,7 @@ void make_coordinate(CoordIndex index, const char* name) {
auto coord = new Coordinates(name);
coords[index] = coord;
if (!coord->load()) {
grbl_msg_sendf(CLIENT_SERIAL, MsgLevel::Info, "Propagating %s data to NVS format", coord->getName());
// If coord->load() returns false it means that no entry
// was present in non-volatile storage. In that case we
// first look for an old-format entry in the EEPROM section.
// If an entry is present some number of float values at
// the beginning of coord_data will be overwritten with
// the EEPROM data, and the rest will remain at 0.0.
// If no old-format entry is present, all will remain 0.0
// Regardless, we create a new entry with that data.
(void)old_settings_read_coord_data(index, coord_data);
coords[index]->set(coord_data);
coords[index]->setDefault();
}
}
void make_settings() {