Skip to content

Commit c0ff593

Browse files
SRGDamia1SRGDamia1
committed
Remove all variable length arrays
Signed-off-by: Sara Damiano <sdamiano@stroudcenter.org>
1 parent 2448aa3 commit c0ff593

File tree

3 files changed

+152
-109
lines changed

3 files changed

+152
-109
lines changed

‎ChangeLog.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,15 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
1212

1313
### Changed
1414

15+
- Removed all variable length arrays within functions
16+
- No longer trimming returned Strings
17+
1518
### Added
1619

20+
- Implemented a single static command buffer
21+
- Added the define `COMMAND_BUFFER_SIZE` to control the size of the command buffer.
22+
- Added `charToRegister(..)` and `charToFrame(..)` functions, each accepting a pointer to a constant char (const char*).
23+
1724
### Removed
1825

1926
### Fixed

‎src/SensorModbusMaster.cpp‎

Lines changed: 89 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414
byte modbusMaster::responseBuffer[RESPONSE_BUFFER_SIZE] = {
1515
0x00,
1616
};
17+
byte modbusMaster::commandBuffer[COMMAND_BUFFER_SIZE] = {
18+
0x00,
19+
};
1720
byte modbusMaster::crcFrame[2] = {
1821
0x00,
1922
};
@@ -299,16 +302,16 @@ bool modbusMaster::pointerToRegister(int regNum, uint16_t value, pointerType poi
299302
return setRegisters(regNum, UINT16_SIZE / 2, inputData, forceMultiple);
300303
}
301304
bool modbusMaster::StringToRegister(int regNum, String value, bool forceMultiple) {
302-
int charLength = value.length();
303-
byte inputData[charLength];
304-
StringToFrame(value, inputData);
305-
return setRegisters(regNum, charLength / 2, inputData, forceMultiple);
305+
return setRegisters(regNum, value.length() / 2, (uint8_t*)value.c_str(),
306+
forceMultiple);
306307
}
307308
bool modbusMaster::charToRegister(int regNum, char inChar[], int charLength,
308309
bool forceMultiple) {
309-
byte inputData[charLength];
310-
charToFrame(inChar, charLength, inputData);
311-
return setRegisters(regNum, charLength / 2, inputData, forceMultiple);
310+
return setRegisters(regNum, charLength / 2, (uint8_t*)inChar, forceMultiple);
311+
}
312+
bool modbusMaster::charToRegister(int regNum, const char* inChar, int charLength,
313+
bool forceMultiple) {
314+
return setRegisters(regNum, charLength / 2, (uint8_t*)inChar, forceMultiple);
312315
}
313316

314317

@@ -450,21 +453,18 @@ int8_t modbusMaster::pointerTypeFromFrame(endianness endian, int start_index) {
450453
}
451454

452455
String modbusMaster::StringFromFrame(int charLength, int start_index) {
453-
char charString[charLength + 1];
454-
int j = 0;
456+
char charString[RESPONSE_BUFFER_SIZE];
457+
memset(charString, '\0', RESPONSE_BUFFER_SIZE);
458+
int j = 0;
455459
for (int i = start_index; i < start_index + charLength; i++) {
456460
// check that it's a printable character
457461
if (responseBuffer[i] >= 0x20 && responseBuffer[i] <= 0x7E) {
458-
// converts from "byte" type to "char" type
462+
// implicitly converts from "byte" type to "char" type
459463
charString[j] = responseBuffer[i];
460464
j++;
461465
}
462466
}
463-
if (j < charLength + 1) {
464-
for (int i = j; i < +charLength + 1; i++) { charString[j] = '\0'; }
465-
}
466467
String string = String(charString);
467-
string.trim();
468468
return string;
469469
}
470470

@@ -473,7 +473,8 @@ void modbusMaster::charFromFrame(char outChar[], int charLength, int start_index
473473
for (int i = start_index; i < start_index + charLength; i++) {
474474
// check that it's a printable character
475475
if (responseBuffer[i] >= 0x20 && responseBuffer[i] <= 0x7E) {
476-
outChar[j] = responseBuffer[i]; // converts from "byte" type to "char" type
476+
// implicitly converts from "byte" type to "char" type
477+
outChar[j] = responseBuffer[i];
477478
j++;
478479
}
479480
}
@@ -639,24 +640,15 @@ void modbusMaster::pointerToFrame(uint16_t value, pointerType point, endianness
639640
}
640641
}
641642
void modbusMaster::StringToFrame(String value, byte modbusFrame[], int start_index) {
642-
int charLength = value.length();
643-
char charString[charLength];
644-
value.toCharArray(charString, charLength);
645-
int j = 0;
646-
for (int i = 0; i < charLength; i++) {
647-
// converts from "char" type to "byte" type
648-
modbusFrame[start_index + j] = charString[i];
649-
j++;
650-
}
643+
memcpy(modbusFrame + start_index, value.c_str(), value.length());
651644
}
652645
void modbusMaster::charToFrame(char inChar[], int charLength, byte modbusFrame[],
653646
int start_index) {
654-
int j = 0;
655-
for (int i = 0; i < charLength; i++) {
656-
// converts from "char" type to "byte" type
657-
modbusFrame[start_index + j] = inChar[i];
658-
j++;
659-
}
647+
memcpy(modbusFrame + start_index, inChar, charLength);
648+
}
649+
void modbusMaster::charToFrame(const char* inChar, int charLength, byte modbusFrame[],
650+
int start_index) {
651+
memcpy(modbusFrame + start_index, inChar, charLength);
660652
}
661653

662654

@@ -686,25 +678,22 @@ bool modbusMaster::getDiscreteInputs(int16_t startInput, int16_t numInputs) {
686678

687679
bool modbusMaster::getModbusData(byte readCommand, int16_t startAddress,
688680
int16_t numChunks, uint8_t expectedReturnBytes) {
689-
// Create an array for the command
690-
byte command[8];
691-
692-
// Put in the slave id and the command
693-
command[0] = _slaveID;
694-
command[1] = readCommand;
681+
// Put in the slave id and the command number into the command buffer
682+
commandBuffer[0] = _slaveID;
683+
commandBuffer[1] = readCommand;
695684

696685
// Put in the starting register
697-
leFrame fram = {{
686+
leFrame fram = {{
698687
0,
699688
}};
700-
fram.Int16[0] = startAddress;
701-
command[2] = fram.Byte[1];
702-
command[3] = fram.Byte[0];
689+
fram.Int16[0] = startAddress;
690+
commandBuffer[2] = fram.Byte[1];
691+
commandBuffer[3] = fram.Byte[0];
703692

704693
// Put in the number of registers
705-
fram.Int16[1] = numChunks;
706-
command[4] = fram.Byte[3];
707-
command[5] = fram.Byte[2];
694+
fram.Int16[1] = numChunks;
695+
commandBuffer[4] = fram.Byte[3];
696+
commandBuffer[5] = fram.Byte[2];
708697

709698
// The size of the returned frame should be:
710699
// # Registers X 1 bit or 2 bytes/register + 5 bytes of modbus RTU frame
@@ -733,11 +722,12 @@ bool modbusMaster::getModbusData(byte readCommand, int16_t startAddress,
733722
// the right slave and has the correct CRC
734723
// The structure of the responses should be:
735724
// {slaveID, fxnCode, # bytes, data, CRC (hi/lo)}
736-
int16_t respSize = sendCommand(command, 8);
725+
int16_t respSize = sendCommand(commandBuffer, 8);
737726
success = (respSize == returnFrameSize &&
738727
responseBuffer[2] == expectedReturnBytes);
739728
// if we got a valid modbusErrorCode, stop trying
740-
if (static_cast<int8_t>(lastError) > 0 && static_cast<int8_t>(lastError) < 0x0C) {
729+
if (static_cast<int8_t>(lastError) > 0 &&
730+
static_cast<int8_t>(lastError) < 0x0C) {
741731
tries = commandRetries; // exit the loop
742732
} else if (!success && lastError == NO_ERROR) { // print error info
743733
debugPrint(F("Failed to get requested data on try "), tries + 1, '\n');
@@ -784,41 +774,42 @@ bool modbusMaster::setRegisters(int16_t startRegister, int16_t numRegisters,
784774
commandLength = 8;
785775
}
786776

787-
// Create an array for the command
788-
byte command[commandLength];
789-
790-
// Put in the slave id and the command
791-
command[0] = _slaveID;
777+
// Put in the slave id and the command number into the command buffer
778+
commandBuffer[0] = _slaveID;
792779
if (numRegisters > 1 || forceMultiple) {
793-
command[1] = 0x10;
780+
commandBuffer[1] = 0x10;
794781
} else {
795-
command[1] = 0x06;
782+
commandBuffer[1] = 0x06;
796783
}
797784

798785
// Put in the starting register
799-
leFrame fram = {{
786+
leFrame fram = {{
800787
0,
801788
}};
802-
fram.Int16[0] = startRegister;
803-
command[2] = fram.Byte[1];
804-
command[3] = fram.Byte[0];
789+
fram.Int16[0] = startRegister;
790+
commandBuffer[2] = fram.Byte[1];
791+
commandBuffer[3] = fram.Byte[0];
805792

806793
// Put in the register values
807794
// For multiple registers, need to add in how many registers and how many bytes
808795
if (numRegisters > 1 || forceMultiple) {
809796
// Put in the number of registers
810-
fram.Int16[1] = numRegisters;
811-
command[4] = fram.Byte[3];
812-
command[5] = fram.Byte[2];
797+
fram.Int16[1] = numRegisters;
798+
commandBuffer[4] = fram.Byte[3];
799+
commandBuffer[5] = fram.Byte[2];
813800
// Put in the number of bytes to write
814-
command[6] = numRegisters * 2;
801+
commandBuffer[6] = numRegisters * 2;
815802
// Put in the data, allowing 7 extra spaces for the modbus frame structure
816-
for (int i = 7; i < numRegisters * 2 + 7; i++) { command[i] = value[i - 7]; }
803+
for (int i = 7; i < numRegisters * 2 + 7; i++) {
804+
commandBuffer[i] = value[i - 7];
805+
}
817806
}
818807
// For a single register, only need the data itself
819808
else {
820809
// Put in the data, allowing 4 extra spaces for the modbus frame structure
821-
for (int i = 4; i < numRegisters * 2 + 4; i++) { command[i] = value[i - 4]; }
810+
for (int i = 4; i < numRegisters * 2 + 4; i++) {
811+
commandBuffer[i] = value[i - 4];
812+
}
822813
}
823814

824815
// Try up to commandRetries times to get the right results
@@ -828,7 +819,7 @@ bool modbusMaster::setRegisters(int16_t startRegister, int16_t numRegisters,
828819
while (!success && tries < commandRetries) {
829820
// Send out the command - this adds the CRC and verifies that the return is from
830821
// the right slave and has the correct CRC
831-
respSize = sendCommand(command, commandLength);
822+
respSize = sendCommand(commandBuffer, commandLength);
832823
// The structure of the response for 0x10 should be:
833824
// {slaveID, fxnCode, Address of 1st register, # Registers, CRC}
834825
if (numRegisters > 1 || forceMultiple) {
@@ -842,7 +833,8 @@ bool modbusMaster::setRegisters(int16_t startRegister, int16_t numRegisters,
842833
responseBuffer[4] == value[0] && responseBuffer[5] == value[1];
843834
}
844835
// if we got a valid modbusErrorCode, stop trying
845-
if (static_cast<int8_t>(lastError) > 0 && static_cast<int8_t>(lastError) < 0x0C) {
836+
if (static_cast<int8_t>(lastError) > 0 &&
837+
static_cast<int8_t>(lastError) < 0x0C) {
846838
tries = commandRetries; // exit the loop
847839
} else if (!success && lastError == NO_ERROR) { // print error info
848840
debugPrint(F("Failed to set register[s] on try "), tries + 1, '\n');
@@ -880,24 +872,21 @@ bool modbusMaster::setCoil(int16_t coilAddress, bool value) {
880872
// For a total size of 8
881873
int commandLength = 8;
882874

883-
// Create an array for the command
884-
byte command[commandLength];
885-
886-
// Put in the slave id and the command
887-
command[0] = _slaveID;
888-
command[1] = 0x05;
875+
// Put in the slave id and the command number into the command buffer
876+
commandBuffer[0] = _slaveID;
877+
commandBuffer[1] = 0x05;
889878

890879
// Put in the coil address
891-
leFrame fram = {{
880+
leFrame fram = {{
892881
0,
893882
}};
894-
fram.Int16[0] = coilAddress;
895-
command[2] = fram.Byte[1];
896-
command[3] = fram.Byte[0];
883+
fram.Int16[0] = coilAddress;
884+
commandBuffer[2] = fram.Byte[1];
885+
commandBuffer[3] = fram.Byte[0];
897886

898887
// Put in the coil value
899-
command[4] = value ? 0xff : 0x00;
900-
command[5] = 0x00;
888+
commandBuffer[4] = value ? 0xff : 0x00;
889+
commandBuffer[5] = 0x00;
901890

902891
// Try up to commandRetries times to get the right results
903892
int tries = 0;
@@ -906,22 +895,24 @@ bool modbusMaster::setCoil(int16_t coilAddress, bool value) {
906895
while (!success && tries < commandRetries) {
907896
// Send out the command - this adds the CRC and verifies that the return is from
908897
// the right slave and has the correct CRC
909-
respSize = sendCommand(command, commandLength);
898+
respSize = sendCommand(commandBuffer, commandLength);
910899
// The structure of the response for 0x05 should be:
911900
// {slaveID, fxnCode, Address of coil (hi/lo), write data hi/lo, CRC (hi/lo)}
912901
// which is exactly the same as the command itself.
913-
if (respSize == 8 && strncmp((char*)responseBuffer, (char*)command, 8) == 0) {
902+
if (respSize == 8 &&
903+
strncmp((char*)responseBuffer, (char*)commandBuffer, 8) == 0) {
914904
success = true;
915905
}
916906
// if we got a valid modbusErrorCode, stop trying
917-
if (static_cast<int8_t>(lastError) > 0 && static_cast<int8_t>(lastError) < 0x0C) {
907+
if (static_cast<int8_t>(lastError) > 0 &&
908+
static_cast<int8_t>(lastError) < 0x0C) {
918909
tries = commandRetries; // exit the loop
919910
} else if (!success && lastError == NO_ERROR) { // print error info
920911
debugPrint(F("Failed to set a single coil on try "), tries + 1, '\n');
921912
debugPrint(F(" Got back "), respSize, F(" of expected "), 8,
922913
F(" bytes from slave\n"));
923914
debugPrint(F(" The slave response "),
924-
strncmp((char*)responseBuffer, (char*)command, 8) == 0
915+
strncmp((char*)responseBuffer, (char*)commandBuffer, 8) == 0
925916
? F("does ")
926917
: F("does not "),
927918
F("match the command\n"));
@@ -946,31 +937,30 @@ bool modbusMaster::setCoils(int16_t startCoil, int16_t numCoils, byte value[]) {
946937
// For a total size of numCoils / 8 + 9
947938
int commandLength = ceil(numCoils / 8.0) + 9;
948939

949-
// Create an array for the command
950-
byte command[commandLength];
951-
952-
// Put in the slave id and the command
953-
command[0] = _slaveID;
954-
command[1] = 0x0F;
940+
// Put in the slave id and the command number in to the command buffer
941+
commandBuffer[0] = _slaveID;
942+
commandBuffer[1] = 0x0F;
955943

956944
// Put in the starting coil
957-
leFrame fram = {{
945+
leFrame fram = {{
958946
0,
959947
}};
960-
fram.Int16[0] = startCoil;
961-
command[2] = fram.Byte[1];
962-
command[3] = fram.Byte[0];
948+
fram.Int16[0] = startCoil;
949+
commandBuffer[2] = fram.Byte[1];
950+
commandBuffer[3] = fram.Byte[0];
963951

964952
// Put in the coil values
965953
// For multiple coils, need to add in how many coils and how many bytes
966954
// Put in the number of coils
967-
fram.Int16[1] = numCoils;
968-
command[4] = fram.Byte[3];
969-
command[5] = fram.Byte[2];
955+
fram.Int16[1] = numCoils;
956+
commandBuffer[4] = fram.Byte[3];
957+
commandBuffer[5] = fram.Byte[2];
970958
// Put in the number of bytes to write
971-
command[6] = ceil(numCoils / 8.0);
959+
commandBuffer[6] = ceil(numCoils / 8.0);
972960
// Put in the data, allowing 7 extra spaces for the modbus frame structure
973-
for (int i = 7; i < ceil(numCoils / 8.0) + 7; i++) { command[i] = value[i - 7]; }
961+
for (int i = 7; i < ceil(numCoils / 8.0) + 7; i++) {
962+
commandBuffer[i] = value[i - 7];
963+
}
974964

975965
// Try up to commandRetries times to get the right results
976966
int tries = 0;
@@ -979,14 +969,15 @@ bool modbusMaster::setCoils(int16_t startCoil, int16_t numCoils, byte value[]) {
979969
while (!success && tries < commandRetries) {
980970
// Send out the command - this adds the CRC and verifies that the return is from
981971
// the right slave and has the correct CRC
982-
respSize = sendCommand(command, commandLength);
972+
respSize = sendCommand(commandBuffer, commandLength);
983973
// The structure of the response for 0x0F should be:
984974
// {slaveID, fxnCode, Address of 1st coil (hi/lo), # coils (hi/lo), CRC
985975
// (hi/lo)}
986976
success = (respSize == 8 && int16FromFrame(bigEndian, 2) == startCoil &&
987977
int16FromFrame(bigEndian, 4) == numCoils);
988978
// if we got a valid modbusErrorCode, stop trying
989-
if (static_cast<int8_t>(lastError) > 0 && static_cast<int8_t>(lastError) < 0x0C) {
979+
if (static_cast<int8_t>(lastError) > 0 &&
980+
static_cast<int8_t>(lastError) < 0x0C) {
990981
tries = commandRetries; // exit the loop
991982
} else if (!success && lastError == NO_ERROR) { // print error info
992983
debugPrint(F("Failed to set multiple coils on try "), tries + 1, '\n');
@@ -1222,7 +1213,7 @@ void modbusMaster::sliceArray(byte inputArray[], byte outputArray[], int start_i
12221213
leFrame modbusMaster::leFrameFromFrame(int varBytes, endianness endian,
12231214
int start_index) {
12241215
// Set up a temporary output frame
1225-
byte outFrame[varBytes];
1216+
byte outFrame[4] = {0, 0, 0, 0};
12261217
// Slice data from the full response frame into the temporary output frame
12271218
if (endian == bigEndian) {
12281219
sliceArray(responseBuffer, outFrame, start_index, varBytes, true);

0 commit comments

Comments
 (0)