Skip to content

Commit

Permalink
Fixed the issues in GenericProtocol that the unit tests brought forward.
Browse files Browse the repository at this point in the history
  • Loading branch information
@atlaste
atlaste committed Jan 20, 2025
1 parent a86a3fe commit 1781499
Showing 6 changed files with 116 additions and 55 deletions.
15 changes: 11 additions & 4 deletions FluidNC/src/Spindles/VFD/DanfossVLT2800Protocol.h
View file
Original file line number Diff line number Diff line change
@@ -33,7 +33,7 @@ namespace Spindles {
union SpindleControl {
struct {
uint8_t reference_preset : 2; // bit 00 = lsb of 2 bit value for preset reference selection
// bit 01 = msb
// bit 01 = msb
bool dc_braking_stop : 1; // bit 02 = 0 causes stop with dc brake
bool coasting_stop : 1; // bit 03 = 0 causes coasting stop
bool quick_stop : 1; // bit 04 = 0 causes quick stop
@@ -46,7 +46,7 @@ namespace Spindles {
bool relay_01 : 1; // bit 11 = 1 activates relay 01
bool output_46 : 1; // bit 12 = 1 activates digital output on terminal 46
uint8_t setup_preset : 2; // bit 13 = lsb of 2 bit value for setup selection when par. 004 multi setup is enabled
// bit 14 = msb
// bit 14 = msb
bool reverse : 1; // bit 15 = 1 causes reversing
} flags;
uint16_t controlWord;
@@ -60,8 +60,15 @@ namespace Spindles {
void direction_command(SpindleState mode, ModbusCommand& data) override;
void set_speed_command(uint32_t rpm, ModbusCommand& data) override;

response_parser initialization_sequence(int index, ModbusCommand& data, VFDSpindle* vfd) {
return get_status_ok_and_init(data, true);
response_parser initialization_sequence(int index, ModbusCommand& data, VFDSpindle* vfd)
{
// This is a bug in the original implementation. Index changes, so we must handle that.
if (index == -1) {
return get_status_ok_and_init(data, true);
}
else {
return nullptr;
}
}
response_parser get_current_speed(ModbusCommand& data) override;
response_parser get_current_direction(ModbusCommand& data) override { return nullptr; };
102 changes: 67 additions & 35 deletions FluidNC/src/Spindles/VFD/GenericProtocol.cpp
View file
Original file line number Diff line number Diff line change
@@ -5,27 +5,28 @@

#include "../VFDSpindle.h"

#include "src/string_util.h"
#include "../../string_util.h"
#include <algorithm>

namespace Spindles {
namespace VFD {
bool split(std::string_view& input, std::string_view& token, const char* delims) {
bool GenericProtocol::split(std::string_view& input, std::string_view& token, const char* delims) {
if (input.size() == 0) {
return false;
}
auto pos = input.find_first_of(delims);
if (pos != std::string_view::npos) {
token = input.substr(0, pos);
input = input.substr(pos + 1);
} else {
}
else {
token = input;
input = "";
}
return true;
}

bool from_decimal(std::string_view str, uint32_t& value) {
bool GenericProtocol::from_decimal(std::string_view str, uint32_t& value) {
value = 0;
if (str.size() == 0) {
return false;
@@ -35,19 +36,20 @@ namespace Spindles {
return false;
}
value = value * 10 + str[0] - '0';
str = str.substr(1);
str = str.substr(1);
}
return true;
}

void scale(uint32_t& n, std::string_view scale_str, uint32_t maxRPM) {
void GenericProtocol::scale(uint32_t& n, std::string_view scale_str, uint32_t maxRPM) {
int32_t divider = 1;
if (scale_str.empty()) {
return;
}
if (scale_str[0] == '%') {
scale_str.remove_prefix(1);
n *= 100;
n /= maxRPM;
divider *= maxRPM;
}
if (scale_str[0] == '*') {
std::string_view numerator_str;
@@ -56,32 +58,38 @@ namespace Spindles {
uint32_t numerator;
if (from_decimal(numerator_str, numerator)) {
n *= numerator;
} else {
}
else {
log_error(spindle->name() << ": bad decimal number " << numerator_str);
return;
}
if (!scale_str.empty()) {
uint32_t denominator;
if (from_decimal(scale_str, denominator)) {
n /= denominator;
} else {
divider *= denominator;
}
else {
log_error(spindle->name() << ": bad decimal number " << scale_str);
return;
}
}
} else if (scale_str[0] == '/') {
}
else if (scale_str[0] == '/') {
std::string_view denominator_str(scale_str.substr(1));
uint32_t denominator;
if (from_decimal(denominator_str, denominator)) {
n /= denominator;
} else {
divider *= denominator;
}
else {
log_error(spindle->name() << ": bad decimal number " << scale_str);
return;
}
}

n /= divider;
}

bool from_xdigit(char c, uint8_t& value) {
bool GenericProtocol::from_xdigit(char c, uint8_t& value) {
if (isdigit(c)) {
value = c - '0';
return true;
@@ -94,7 +102,7 @@ namespace Spindles {
return false;
}

bool from_hex(std::string_view str, uint8_t& value) {
bool GenericProtocol::from_hex(std::string_view str, uint8_t& value) {
value = 0;
if (str.size() == 0 || str.size() > 2) {
return false;
@@ -110,9 +118,9 @@ namespace Spindles {
}
return true;
}
bool set_data(std::string_view token, std::basic_string_view<uint8_t>& response_view, const char* name, uint32_t& data) {
bool GenericProtocol::set_data(std::string_view token, std::basic_string_view<uint8_t>& response_view, const char* name, uint32_t& data) {
if (string_util::starts_with_ignore_case(token, name)) {
uint32_t rval = (response_view[0] << 8) + (response_view[1] & 0xff);
uint32_t rval = (response_view[0] << 8) + (response_view[1] & 0xff);
uint32_t orval = rval;
scale(rval, token.substr(strlen(name)), 1);
data = rval;
@@ -134,9 +142,17 @@ namespace Spindles {
// Ignore repeated blanks
continue;
}
if (set_data(token, response_view, "rpm", spindle->_sync_dev_speed)) {

// Sync must be in a temporary because it's volatile!
uint32_t sync = spindle->_sync_dev_speed;
if (set_data(token, response_view, "rpm", sync)) {
spindle->_sync_dev_speed = sync;
continue;
}
uint32_t ignore;
if (set_data(token, response_view, "ignore", ignore)) {
continue;
}
if (set_data(token, response_view, "minrpm", instance->_minRPM)) {
log_debug(spindle->name() << ": got minRPM " << instance->_minRPM);
continue;
@@ -181,9 +197,11 @@ namespace Spindles {
scale(out, token.substr(strlen("rpm")), _maxRPM);
data.msg[data.tx_length++] = out >> 8;
data.msg[data.tx_length++] = out & 0xff;
} else if (from_hex(token, data.msg[data.tx_length])) {
}
else if (from_hex(token, data.msg[data.tx_length])) {
++data.tx_length;
} else {
}
else {
log_error(spindle->name() << ":Bad hex number " << token);
return;
}
@@ -199,26 +217,28 @@ namespace Spindles {
break;
}
if (string_util::starts_with_ignore_case(token, "rpm") || string_util::starts_with_ignore_case(token, "minrpm") ||
string_util::starts_with_ignore_case(token, "maxrpm")) {
string_util::starts_with_ignore_case(token, "maxrpm") || string_util::starts_with_ignore_case(token, "ignore")) {
data.rx_length += 2;
} else if (from_hex(token, x)) {
}
else if (from_hex(token, x)) {
++data.rx_length;
} else {
}
else {
log_error(spindle->name() << ": bad hex number " << token);
}
}
}
void GenericProtocol::direction_command(SpindleState mode, ModbusCommand& data) {
switch (mode) {
case SpindleState::Cw:
send_vfd_command(_cw_cmd, data, 0);
break;
case SpindleState::Ccw:
send_vfd_command(_ccw_cmd, data, 0);
break;
default: // SpindleState::Disable
send_vfd_command(_off_cmd, data, 0);
break;
case SpindleState::Cw:
send_vfd_command(_cw_cmd, data, 0);
break;
case SpindleState::Ccw:
send_vfd_command(_ccw_cmd, data, 0);
break;
default: // SpindleState::Disable
send_vfd_command(_off_cmd, data, 0);
break;
}
}

@@ -231,7 +251,7 @@ namespace Spindles {
return [](const uint8_t* response, VFDSpindle* spindle, VFDProtocol* protocol) -> bool {
auto instance = static_cast<GenericProtocol*>(protocol);
return instance->parser(response, spindle, instance);
};
};
}

void GenericProtocol::setup_speeds(VFDSpindle* vfd) {
@@ -240,19 +260,31 @@ namespace Spindles {
vfd->_slop = 300;
}
VFDProtocol::response_parser GenericProtocol::initialization_sequence(int index, ModbusCommand& data, VFDSpindle* vfd) {
// BUG:
//
// If we do:
// _get_min_rpm_cmd = "03 00 0B 00 01 > 03 02 maxrpm*60";
// _get_max_rpm_cmd = "03 00 05 00 01 > 03 02 maxrpm*60";
//
// then the minrpm will never be assigned, and we end up in an infinite loop.
//
// NOT FIXED. I'm not really sure what the best approach is. Perhaps check if after the sequence
// something changed?

this->spindle = vfd;
if (_maxRPM == 0xffffffff && !_get_max_rpm_cmd.empty()) {
send_vfd_command(_get_max_rpm_cmd, data, 0);
return [](const uint8_t* response, VFDSpindle* spindle, VFDProtocol* protocol) -> bool {
auto instance = static_cast<GenericProtocol*>(protocol);
return instance->parser(response, spindle, instance);
};
};
}
if (_minRPM == 0xffffffff && !_get_min_rpm_cmd.empty()) {
send_vfd_command(_get_min_rpm_cmd, data, 0);
return [](const uint8_t* response, VFDSpindle* spindle, VFDProtocol* protocol) -> bool {
auto instance = static_cast<GenericProtocol*>(protocol);
return instance->parser(response, spindle, instance);
};
};
}
if (vfd->_speeds.size() == 0) {
setup_speeds(vfd);
24 changes: 23 additions & 1 deletion FluidNC/src/Spindles/VFD/GenericProtocol.h
View file
Original file line number Diff line number Diff line change
@@ -4,11 +4,30 @@
#pragma once

#include "VFDProtocol.h"
#include <string_view>


namespace Testing {
class GenericProtocolTest;
}

namespace Spindles {
class VFDSpindle;

namespace VFD {
class GenericProtocol : public VFDProtocol, Configuration::Configurable {
private:
friend class ::Testing::GenericProtocolTest;

bool split(std::string_view& input, std::string_view& token, const char* delims);
bool from_decimal(std::string_view str, uint32_t& value);
void scale(uint32_t& n, std::string_view scale_str, uint32_t maxRPM);
bool from_xdigit(char c, uint8_t& value);
bool from_hex(std::string_view str, uint8_t& value);
bool set_data(std::string_view token, std::basic_string_view<uint8_t>& response_view, const char* name, uint32_t& data);

protected:

void direction_command(SpindleState mode, ModbusCommand& data) override;
void set_speed_command(uint32_t dev_speed, ModbusCommand& data) override;

@@ -30,9 +49,12 @@ namespace Spindles {

private:
std::string _model; // VFD Model name
uint32_t* _response_data;
uint32_t* _response_data;
uint32_t _minRPM = 0xffffffff;
uint32_t _maxRPM = 0xffffffff;

VFDSpindle* spindle;

bool parser(const uint8_t* response, VFDSpindle* spindle, GenericProtocol* protocol);
void send_vfd_command(const std::string cmd, ModbusCommand& data, uint32_t out);
std::string _response_format;
2 changes: 1 addition & 1 deletion FluidNC/src/Spindles/VFD/H2AProtocol.cpp
View file
Original file line number Diff line number Diff line change
@@ -67,7 +67,7 @@ namespace Spindles {
data.msg[3] = 0x05;
data.msg[4] = 0x00; // Read 2 values
data.msg[5] = 0x02;

// Recv: 01 03 00 04 5D C0 03 F6
// -- -- = 24000 (val #1)
return [](const uint8_t* response, VFDSpindle* vfd, VFDProtocol* detail) -> bool {
10 changes: 5 additions & 5 deletions FluidNC/src/Spindles/VFD/NowForeverProtocol.cpp
View file
Original file line number Diff line number Diff line change
@@ -81,12 +81,12 @@ namespace Spindles {
data.msg[5] = 0x02; // Number of elements, low byte (2 elements)

/*
Contents of register 0x0007
Bit 0-15: max speed in hz * 100
Contents of register 0x0007
Bit 0-15: max speed in hz * 100
Contents of register 0x0008
Bit 0-15: min speed in hz * 100
*/
Contents of register 0x0008
Bit 0-15: min speed in hz * 100
*/

return [](const uint8_t* response, VFDSpindle* vfd, VFDProtocol* detail) -> bool {
if (response[1] != 0x03) {
18 changes: 9 additions & 9 deletions FluidNC/src/Spindles/VFD/SiemensV20Protocol.cpp
View file
Original file line number Diff line number Diff line change
@@ -151,11 +151,11 @@ namespace Spindles {
<< _maxFrequency << ")");
}
/*
V20 has a scalled input and is standardized to 16384
please note Signed numbers work IE -16384 to 16384
but for this implementation only posivite number are allowed
*/
int16_t ScaledFreq = speed * _FreqScaler;
V20 has a scalled input and is standardized to 16384
please note Signed numbers work IE -16384 to 16384
but for this implementation only posivite number are allowed
*/
int16_t ScaledFreq = int16_t(speed * _FreqScaler);
log_debug("Setting VFD Scaled Value " << int16_t(ScaledFreq) << " Byte 1 " << uint8_t(ScaledFreq >> 8) << " Byte 2 "
<< uint8_t(ScaledFreq & 0xFF));

@@ -183,9 +183,9 @@ namespace Spindles {

return [](const uint8_t* response, VFDSpindle* vfd, VFDProtocol* detail) -> bool {
/*
The VFD does not have any noticeable registers to set this information up programmatically
For now - it is user set in the software but is a typical setup
*/
The VFD does not have any noticeable registers to set this information up programmatically
For now - it is user set in the software but is a typical setup
*/
auto siemens = static_cast<SiemensV20Protocol*>(detail);

if (siemens->_minFrequency > siemens->_maxFrequency) {
@@ -218,7 +218,7 @@ namespace Spindles {
return [](const uint8_t* response, VFDSpindle* vfd, VFDProtocol* detail) -> bool {
auto siemensV20 = static_cast<SiemensV20Protocol*>(detail);
int16_t Scaledfrequency = ((response[3] << 8) | response[4]);
int16_t frequency = float(Scaledfrequency) / (-1 * (siemensV20->_FreqScaler));
int16_t frequency = int16_t(float(Scaledfrequency) / (-1 * (siemensV20->_FreqScaler)));
log_debug("VFD Measured Value " << int16_t(Scaledfrequency) << " Freq " << int16_t(frequency));

// Store speed for synchronization

0 comments on commit 1781499

Please sign in to comment.