DeviceSettings: Remove checksum validation

pull/76/merge
Kyle Schwarz 2025-10-06 15:36:55 -04:00
parent 895cd0792c
commit ec350b522a
6 changed files with 14 additions and 43 deletions

View File

@ -40,7 +40,7 @@ void init_idevicesettings(pybind11::module_& m) {
.def("set_phy_enable", &IDeviceSettings::setPhyEnable, pybind11::call_guard<pybind11::gil_scoped_release>()) .def("set_phy_enable", &IDeviceSettings::setPhyEnable, pybind11::call_guard<pybind11::gil_scoped_release>())
.def("set_phy_mode", &IDeviceSettings::setPhyMode, pybind11::call_guard<pybind11::gil_scoped_release>()) .def("set_phy_mode", &IDeviceSettings::setPhyMode, pybind11::call_guard<pybind11::gil_scoped_release>())
.def("set_phy_speed", &IDeviceSettings::setPhySpeed, pybind11::call_guard<pybind11::gil_scoped_release>()) .def("set_phy_speed", &IDeviceSettings::setPhySpeed, pybind11::call_guard<pybind11::gil_scoped_release>())
.def("refresh", &IDeviceSettings::refresh, pybind11::arg("ignoreChecksum") = 0, pybind11::call_guard<pybind11::gil_scoped_release>()); .def("refresh", &IDeviceSettings::refresh, pybind11::call_guard<pybind11::gil_scoped_release>());
} }
} // namespace icsneo } // namespace icsneo

View File

@ -4,9 +4,9 @@
using namespace icsneo; using namespace icsneo;
std::optional<uint16_t> IDeviceSettings::CalculateGSChecksum(const std::vector<uint8_t>& settings, std::optional<size_t> knownSize) { std::optional<uint16_t> IDeviceSettings::CalculateGSChecksum(const std::vector<uint8_t>& settings) {
const uint16_t* p = reinterpret_cast<const uint16_t*>(settings.data()); const uint16_t* p = reinterpret_cast<const uint16_t*>(settings.data());
size_t words = std::min(knownSize.value_or(0), settings.size()); size_t words = settings.size();
if(words % 2 == 1) if(words % 2 == 1)
return std::nullopt; // Somehow settings is not word aligned return std::nullopt; // Somehow settings is not word aligned
words /= 2; words /= 2;
@ -159,15 +159,12 @@ bool IDeviceSettings::ValidateLINBaudrate(int64_t baudrate) {
} }
} }
bool IDeviceSettings::refresh(bool ignoreChecksum) { bool IDeviceSettings::refresh() {
if(disabled) { if(disabled) {
report(APIEvent::Type::SettingsNotAvailable, APIEvent::Severity::Error); report(APIEvent::Type::SettingsNotAvailable, APIEvent::Severity::Error);
return false; return false;
} }
if(disableGSChecksumming)
ignoreChecksum = true;
std::vector<uint8_t> rxSettings; std::vector<uint8_t> rxSettings;
bool ret = com->getSettingsSync(rxSettings); bool ret = com->getSettingsSync(rxSettings);
if(!ret) { if(!ret) {
@ -175,24 +172,14 @@ bool IDeviceSettings::refresh(bool ignoreChecksum) {
return false; return false;
} }
constexpr size_t GsSize = 3 * sizeof(uint16_t); constexpr size_t GsSize = 3 * sizeof(uint16_t); // <version> <length> <checksum>
if(rxSettings.size() < GsSize) { // We need to at least have the header of GLOBAL_SETTINGS if(rxSettings.size() < GsSize) { // We need to at least have the header of GLOBAL_SETTINGS
report(APIEvent::Type::SettingsReadError, APIEvent::Severity::Error); report(APIEvent::Type::SettingsReadError, APIEvent::Severity::Error);
return false; return false;
} }
// The length of the settings structure sent to us
// This is the length the firmware thinks the current version of the structure is
const size_t rxLen = rxSettings.size() - GsSize;
const uint16_t gsVersion = rxSettings[0] | (rxSettings[1] << 8); const uint16_t gsVersion = rxSettings[0] | (rxSettings[1] << 8);
// The length of the settings last saved
// If the firmware is updated, it will have either extended (with zeros) or truncated
// the structure, but this value will continue to be set to the last saved value
const uint16_t gsLen = rxSettings[2] | (rxSettings[3] << 8);
const uint16_t gsChecksum = rxSettings[4] | (rxSettings[5] << 8);
rxSettings.erase(rxSettings.begin(), rxSettings.begin() + GsSize); rxSettings.erase(rxSettings.begin(), rxSettings.begin() + GsSize);
if(gsVersion != GS_VERSION) { if(gsVersion != GS_VERSION) {
@ -200,19 +187,6 @@ bool IDeviceSettings::refresh(bool ignoreChecksum) {
return false; return false;
} }
if(rxLen < gsLen) {
// We got less data, i.e. the firmware thinks the strucure is smaller than what
// was last saved. Usually this is due to a firmware downgrade. We'll ignore the
// checksum for now, because it will definitely be wrong.
ignoreChecksum = true;
}
// We check the checksum against the data last saved
if(!ignoreChecksum && gsChecksum != CalculateGSChecksum(rxSettings, gsLen)) {
report(APIEvent::Type::SettingsChecksumError, APIEvent::Severity::Error);
return false;
}
settings = std::move(rxSettings); settings = std::move(rxSettings);
settingsInDeviceRAM = settings; settingsInDeviceRAM = settings;
settingsLoaded = true; settingsLoaded = true;
@ -261,7 +235,7 @@ bool IDeviceSettings::apply(bool temporary) {
std::shared_ptr<Main51Message> msg = std::dynamic_pointer_cast<Main51Message>(com->waitForMessageSync([this, &bytestream]() { std::shared_ptr<Main51Message> msg = std::dynamic_pointer_cast<Main51Message>(com->waitForMessageSync([this, &bytestream]() {
return com->sendCommand(Command::SetSettings, bytestream); return com->sendCommand(Command::SetSettings, bytestream);
}, std::make_shared<Main51MessageFilter>(Command::SetSettings), std::chrono::milliseconds(1000))); }, std::make_shared<Main51MessageFilter>(Command::SetSettings), std::chrono::milliseconds(2000)));
if(!msg || msg->data[0] != 1) { // We did not receive a response if(!msg || msg->data[0] != 1) { // We did not receive a response
// Attempt to get the settings from the device so we're up to date if possible // Attempt to get the settings from the device so we're up to date if possible
@ -272,7 +246,7 @@ bool IDeviceSettings::apply(bool temporary) {
return false; return false;
} }
refresh(true); // Refresh ignoring checksum refresh();
// The device might modify the settings once they are applied, however in this case it does not update the checksum // The device might modify the settings once they are applied, however in this case it does not update the checksum
// We refresh to get these updates, update the checksum, and send it back so it's all in sync // We refresh to get these updates, update the checksum, and send it back so it's all in sync
gsChecksum = CalculateGSChecksum(settings); gsChecksum = CalculateGSChecksum(settings);
@ -287,7 +261,7 @@ bool IDeviceSettings::apply(bool temporary) {
msg = std::dynamic_pointer_cast<Main51Message>(com->waitForMessageSync([this, &bytestream]() { msg = std::dynamic_pointer_cast<Main51Message>(com->waitForMessageSync([this, &bytestream]() {
return com->sendCommand(Command::SetSettings, bytestream); return com->sendCommand(Command::SetSettings, bytestream);
}, std::make_shared<Main51MessageFilter>(Command::SetSettings), std::chrono::milliseconds(1000))); }, std::make_shared<Main51MessageFilter>(Command::SetSettings), std::chrono::milliseconds(2000)));
if(!msg || msg->data[0] != 1) { if(!msg || msg->data[0] != 1) {
// Attempt to get the settings from the device so we're up to date if possible // Attempt to get the settings from the device so we're up to date if possible
if(refresh()) { if(refresh()) {
@ -342,7 +316,7 @@ bool IDeviceSettings::applyDefaults(bool temporary) {
// This short wait helps on FIRE devices, otherwise the checksum might be wrong! // This short wait helps on FIRE devices, otherwise the checksum might be wrong!
std::this_thread::sleep_for(std::chrono::milliseconds(3)); std::this_thread::sleep_for(std::chrono::milliseconds(3));
refresh(true); // Refresh ignoring checksum refresh();
// The device might modify the settings once they are applied, however in this case it does not update the checksum // The device might modify the settings once they are applied, however in this case it does not update the checksum
// We refresh to get these updates, update the checksum, and send it back so it's all in sync // We refresh to get these updates, update the checksum, and send it back so it's all in sync
std::vector<uint8_t> bytestream; std::vector<uint8_t> bytestream;
@ -364,7 +338,7 @@ bool IDeviceSettings::applyDefaults(bool temporary) {
msg = std::dynamic_pointer_cast<Main51Message>(com->waitForMessageSync([this, &bytestream]() { msg = std::dynamic_pointer_cast<Main51Message>(com->waitForMessageSync([this, &bytestream]() {
return com->sendCommand(Command::SetSettings, bytestream); return com->sendCommand(Command::SetSettings, bytestream);
}, std::make_shared<Main51MessageFilter>(Command::SetSettings), std::chrono::milliseconds(1000))); }, std::make_shared<Main51MessageFilter>(Command::SetSettings), std::chrono::milliseconds(2000)));
if(!msg || msg->data[0] != 1) { if(!msg || msg->data[0] != 1) {
// Attempt to get the settings from the device so we're up to date if possible // Attempt to get the settings from the device so we're up to date if possible
if(refresh()) { if(refresh()) {

View File

@ -729,7 +729,7 @@ public:
using TerminationGroup = std::vector<Network>; using TerminationGroup = std::vector<Network>;
static constexpr uint16_t GS_VERSION = 5; static constexpr uint16_t GS_VERSION = 5;
static std::optional<uint16_t> CalculateGSChecksum(const std::vector<uint8_t>& settings, std::optional<size_t> knownSize = std::nullopt); static std::optional<uint16_t> CalculateGSChecksum(const std::vector<uint8_t>& settings);
static CANBaudrate GetEnumValueForBaudrate(int64_t baudrate); static CANBaudrate GetEnumValueForBaudrate(int64_t baudrate);
static int64_t GetBaudrateValueForEnum(CANBaudrate enumValue); static int64_t GetBaudrateValueForEnum(CANBaudrate enumValue);
static bool ValidateLINBaudrate(int64_t baudrate); static bool ValidateLINBaudrate(int64_t baudrate);
@ -738,7 +738,7 @@ public:
virtual ~IDeviceSettings() {} virtual ~IDeviceSettings() {}
bool ok() const { return !disabled && settingsLoaded; } bool ok() const { return !disabled && settingsLoaded; }
virtual bool refresh(bool ignoreChecksum = false); // Get from device virtual bool refresh(); // Get from device
// Send to device, if temporary device keeps settings in volatile RAM until power cycle, otherwise saved to EEPROM // Send to device, if temporary device keeps settings in volatile RAM until power cycle, otherwise saved to EEPROM
virtual bool apply(bool temporary = false); virtual bool apply(bool temporary = false);
@ -946,7 +946,6 @@ public:
bool disabled = false; bool disabled = false;
bool readonly = false; bool readonly = false;
bool disableGSChecksumming = false;
std::atomic<bool> applyingSettings{false}; std::atomic<bool> applyingSettings{false};
protected: protected:

View File

@ -75,7 +75,6 @@ static_assert(sizeof(radpluto_settings_t) == 322, "RAD-Pluto Settings are not pa
class RADPlutoSettings : public IDeviceSettings { class RADPlutoSettings : public IDeviceSettings {
public: public:
RADPlutoSettings(std::shared_ptr<Communication> com) : IDeviceSettings(com, sizeof(radpluto_settings_t)) { RADPlutoSettings(std::shared_ptr<Communication> com) : IDeviceSettings(com, sizeof(radpluto_settings_t)) {
disableGSChecksumming = true;
} }
const CAN_SETTINGS* getCANSettingsFor(Network net) const override { const CAN_SETTINGS* getCANSettingsFor(Network net) const override {

View File

@ -70,7 +70,6 @@ typedef struct {
class RADStar2Settings : public IDeviceSettings { class RADStar2Settings : public IDeviceSettings {
public: public:
RADStar2Settings(std::shared_ptr<Communication> com) : IDeviceSettings(com, sizeof(radstar2_settings_t)) { RADStar2Settings(std::shared_ptr<Communication> com) : IDeviceSettings(com, sizeof(radstar2_settings_t)) {
disableGSChecksumming = true;
} }
const CAN_SETTINGS* getCANSettingsFor(Network net) const override { const CAN_SETTINGS* getCANSettingsFor(Network net) const override {

View File

@ -89,11 +89,11 @@ public:
}; };
} }
bool refresh(bool ignoreChecksum = false) override { bool refresh() override {
// Because VividCAN uses a nonstandard 16-bit termination_enables // Because VividCAN uses a nonstandard 16-bit termination_enables
// we need to keep the standard 64-bit values in memory and update // we need to keep the standard 64-bit values in memory and update
// the structure when applying // the structure when applying
if(!IDeviceSettings::refresh(ignoreChecksum)) if(!IDeviceSettings::refresh())
return false; return false;
auto cfg = getStructurePointer<vividcan_settings_t>(); auto cfg = getStructurePointer<vividcan_settings_t>();
if(cfg == nullptr) if(cfg == nullptr)