main() calls parseArgs() which populates global ttyPort/baudRate, but MCUDeviceCommunication::MCUDeviceCommunication() hardcodes the constants directly:
// ProtocolComm.cpp:74 — ignores ttyPort and baudRate globals entirely
DeviceCommWithMCU(MCU_DEV_COMM_UART_DEVICE_NAME, MCU_DEV_COMM_UART_DEVICE_BAUD_RATE, ...)
The -p and -b flags in the service ExecStart do nothing.
Bug: Iteration count doesn't equal full cycles
mcupep.cpp:43:
int i = readAttempts + 1; // = 6 with -c 5
while(i-- > 0) { ... state machine ... }
One full send/receive cycle requires 4 state transitions (SEND → GET → PUB → DELAY). With i=6 the loop exits mid-cycle 2: SEND(1)→GET(2)→PUB(3)→DELAY(4)→SEND(5)→GET(6). The -c parameter doesn't configure the number of complete cycles.
Bug: TIMESTAMP_STATUS parses only 4 of 8 bytes, little-endian mismatch
ProtocolComm.cpp:279:
SymbolStatusData.TimestampStatus = StatusDataPacket[5]<<24 | StatusDataPacket[4]<<16 | StatusDataPacket[3]<<8 | StatusDataPacket[2];
MCU_DEV_TIMESTAMP_STATUS_DATA_BYTE_SIZE = 8, but only 4 bytes are read. The ReadBigEndian64 helper exists but isn't used here. Also setSystemTimeFromUnix() is commented out, so the timestamp is parsed but never applied.
Dead code: hardcoded ping variables
ProtocolComm.cpp:268-286 — ping = 0, the GetData() call is commented out, so the if(ping != 1) branch always executes and the else { RetStatus = true; } is unreachable.
ProtocolComm.cpp:425-436 — ping = 1 hardcoded, the else branch is unreachable.
Service has no ordering dependency
mcupep.service has no Before=mvcamera.service or After=initmodem.service. If the point is to initialize MCU before streaming starts, there's no guarantee of ordering.
Debug output going to stdout instead of glog
ProtocolComm.cpp is full of printf and std::cout calls. GLOG_log_dir=/var/log is set in the service environment but most output bypasses glog entirely. Also bool dbg = true is hardcoded in McuTest() at mcupep.cpp:59.
Minor: unused field + typo
MCUWriteState MCUWriteStateStatus declared in the class (ProtocolComm.hpp:455) but never read — a static local is used in SendDeviceStatusDataToMCU instead.
ACCELEROMETER_SENSSITIVITY_STATUS — double 'S' in both the enum and switch.
Makefile declares glib-2.0 and json-glib-1.0 as deps but neither is used.
PR: https://lab.rclmx.ru/gogs/MView/5gcamera/pulls/78
## Bug: Command-line args have no effect on UART
main() calls parseArgs() which populates global ttyPort/baudRate, but MCUDeviceCommunication::MCUDeviceCommunication() hardcodes the constants directly:
// ProtocolComm.cpp:74 — ignores ttyPort and baudRate globals entirely
DeviceCommWithMCU(MCU_DEV_COMM_UART_DEVICE_NAME, MCU_DEV_COMM_UART_DEVICE_BAUD_RATE, ...)
The -p and -b flags in the service ExecStart do nothing.
## Bug: Iteration count doesn't equal full cycles
mcupep.cpp:43:
int i = readAttempts + 1; // = 6 with -c 5
while(i-- > 0) { ... state machine ... }
One full send/receive cycle requires 4 state transitions (SEND → GET → PUB → DELAY). With i=6 the loop exits mid-cycle 2: SEND(1)→GET(2)→PUB(3)→DELAY(4)→SEND(5)→GET(6). The -c parameter doesn't configure the number of complete cycles.
## Bug: TIMESTAMP_STATUS parses only 4 of 8 bytes, little-endian mismatch
ProtocolComm.cpp:279:
SymbolStatusData.TimestampStatus = StatusDataPacket[5]<<24 | StatusDataPacket[4]<<16 | StatusDataPacket[3]<<8 | StatusDataPacket[2];
MCU_DEV_TIMESTAMP_STATUS_DATA_BYTE_SIZE = 8, but only 4 bytes are read. The ReadBigEndian64 helper exists but isn't used here. Also setSystemTimeFromUnix() is commented out, so the timestamp is parsed but never applied.
## Dead code: hardcoded ping variables
ProtocolComm.cpp:268-286 — ping = 0, the GetData() call is commented out, so the if(ping != 1) branch always executes and the else { RetStatus = true; } is unreachable.
ProtocolComm.cpp:425-436 — ping = 1 hardcoded, the else branch is unreachable.
## Service has no ordering dependency
mcupep.service has no Before=mvcamera.service or After=initmodem.service. If the point is to initialize MCU before streaming starts, there's no guarantee of ordering.
## Debug output going to stdout instead of glog
ProtocolComm.cpp is full of printf and std::cout calls. GLOG_log_dir=/var/log is set in the service environment but most output bypasses glog entirely. Also bool dbg = true is hardcoded in McuTest() at mcupep.cpp:59.
## Minor: unused field + typo
MCUWriteState MCUWriteStateStatus declared in the class (ProtocolComm.hpp:455) but never read — a static local is used in SendDeviceStatusDataToMCU instead.
ACCELEROMETER_SENSSITIVITY_STATUS — double 'S' in both the enum and switch.
Makefile declares glib-2.0 and json-glib-1.0 as deps but neither is used.
main() calls parseArgs(), which populates global ttyPort/baudRate. Global ttyPort/baudRate is used in another SerialPort file...
Action: ttyPort/baudRate removed them from ProtocolComm.cpp
...but MCUDeviceCommunication::MCUDeviceCommunication() hardcodes the constants directly:
// ProtocolComm.cpp:74 — ignores ttyPort and baudRate globals entirely DeviceCommWithMCU(MCU_DEV_COMM_UART_DEVICE_NAME, MCU_DEV_COMM_UART_DEVICE_BAUD_RATE, ...) The -p and -b flags in the service ExecStart do nothing.
Analysis: MCUDeviceCommunication() constructor is initialized with default values
Action: Do nothing
Bug: Iteration count doesn't equal full cycles
mcupep.cpp:43:
int i = readAttempts + 1; // = 6 with -c 5 while(i-- > 0) { ... state machine ... } One full send/receive cycle requires 4 state transitions (SEND → GET → PUB → DELAY). With i=6 the loop exits mid-cycle 2: SEND(1)→GET(2)→PUB(3)→DELAY(4)→SEND(5)→GET(6). The -c parameter doesn't configure the number of complete cycles.
Analysis: For completeness testing, only one successful read cycle matters. The original is an infinite loop. We need a limited number of cycles.
Action: I fixed this by making two successful cycles the default.
Bug: TIMESTAMP_STATUS parses only 4 of 8 bytes, little-endian mismatch
ProtocolComm.cpp:279:
SymbolStatusData.TimestampStatus = StatusDataPacket[5]<<24 | StatusDataPacket[4]<<16 | StatusDataPacket[3]<<8 | StatusDataPacket[2]; MCU_DEV_TIMESTAMP_STATUS_DATA_BYTE_SIZE = 8, but only 4 bytes are read. The ReadBigEndian64 helper exists but isn't used here. Also setSystemTimeFromUnix() is commented out, so the timestamp is parsed but never applied.
Anallysis: Good findinig
Action: Fixed.
Dead code: hardcoded ping variables
ProtocolComm.cpp:268-286 — ping = 0, the GetData() call is commented out, so the if(ping != 1) branch always executes and the else { RetStatus = true; } is unreachable.
ProtocolComm.cpp:425-436 — ping = 1 hardcoded, the else branch is unreachable.
Action: Extra code removed.
Service has no ordering dependency
mcupep.service has no Before=mvcamera.service or After=initmodem.service. If the point is to initialize MCU before streaming starts, there's no guarantee of ordering.
Analysis: The order doesn't matter. The main thing is that the service starts within the first 30-60 seconds.
Action: Do nothing.
Debug output going to stdout instead of glog
ProtocolComm.cpp is full of printf and std::cout calls. GLOG_log_dir=/var/log is set in the service environment but most output bypasses glog entirely. Also bool dbg = true is hardcoded in McuTest() at mcupep.cpp:59.
Analysis: bool dbg = true is for testing, it is not finished yet.
Action: Do nothing
Minor: unused field + typo
MCUWriteState MCUWriteStateStatus declared in the class (ProtocolComm.hpp:455) but never read — a static local is used in SendDeviceStatusDataToMCU instead. ACCELEROMETER_SENSSITIVITY_STATUS — double 'S' in both the enum and switch. Makefile declares glib-2.0 and json-glib-1.0 as deps but neither is used.
Action: glib-2.0 and json-glib-1.0 removed from Makefile
## Bug: Command-line args have no effect on UART ##
Conclusion: False AI trigger
* main() calls parseArgs(), which populates global ttyPort/baudRate. Global ttyPort/baudRate is used in another SerialPort file...
Action: ttyPort/baudRate removed them from ProtocolComm.cpp
...but MCUDeviceCommunication::MCUDeviceCommunication() hardcodes the constants directly:
* // ProtocolComm.cpp:74 — ignores ttyPort and baudRate globals entirely DeviceCommWithMCU(MCU_DEV_COMM_UART_DEVICE_NAME, MCU_DEV_COMM_UART_DEVICE_BAUD_RATE, ...) The -p and -b flags in the service ExecStart do nothing.
Analysis: MCUDeviceCommunication() constructor is initialized with default values
Action: Do nothing
## Bug: Iteration count doesn't equal full cycles ##
* mcupep.cpp:43:
int i = readAttempts + 1; // = 6 with -c 5 while(i-- > 0) { ... state machine ... } One full send/receive cycle requires 4 state transitions (SEND → GET → PUB → DELAY). With i=6 the loop exits mid-cycle 2: SEND(1)→GET(2)→PUB(3)→DELAY(4)→SEND(5)→GET(6). The -c parameter doesn't configure the number of complete cycles.
Analysis: For completeness testing, only one successful read cycle matters. The original is an infinite loop. We need a limited number of cycles.
Action: I fixed this by making two successful cycles the default.
## Bug: TIMESTAMP_STATUS parses only 4 of 8 bytes, little-endian mismatch ##
ProtocolComm.cpp:279:
SymbolStatusData.TimestampStatus = StatusDataPacket[5]<<24 | StatusDataPacket[4]<<16 | StatusDataPacket[3]<<8 | StatusDataPacket[2]; MCU_DEV_TIMESTAMP_STATUS_DATA_BYTE_SIZE = 8, but only 4 bytes are read. The ReadBigEndian64 helper exists but isn't used here. Also setSystemTimeFromUnix() is commented out, so the timestamp is parsed but never applied.
Anallysis: Good findinig
Action: Fixed.
## Dead code: hardcoded ping variables ##
ProtocolComm.cpp:268-286 — ping = 0, the GetData() call is commented out, so the if(ping != 1) branch always executes and the else { RetStatus = true; } is unreachable.
ProtocolComm.cpp:425-436 — ping = 1 hardcoded, the else branch is unreachable.
Action: Extra code removed.
## Service has no ordering dependency ##
mcupep.service has no Before=mvcamera.service or After=initmodem.service. If the point is to initialize MCU before streaming starts, there's no guarantee of ordering.
Analysis: The order doesn't matter. The main thing is that the service starts within the first 30-60 seconds.
Action: Do nothing.
## Debug output going to stdout instead of glog ##
ProtocolComm.cpp is full of printf and std::cout calls. GLOG_log_dir=/var/log is set in the service environment but most output bypasses glog entirely. Also bool dbg = true is hardcoded in McuTest() at mcupep.cpp:59.
Analysis: bool dbg = true is for testing, it is not finished yet.
Action: Do nothing
## Minor: unused field + typo ##
MCUWriteState MCUWriteStateStatus declared in the class (ProtocolComm.hpp:455) but never read — a static local is used in SendDeviceStatusDataToMCU instead. ACCELEROMETER_SENSSITIVITY_STATUS — double 'S' in both the enum and switch. Makefile declares glib-2.0 and json-glib-1.0 as deps but neither is used.
Action: glib-2.0 and json-glib-1.0 removed from Makefile
sgolubev
PR: https://lab.rclmx.ru/gogs/MView/5gcamera/pulls/78
Bug: Command-line args have no effect on UART
main() calls parseArgs() which populates global ttyPort/baudRate, but MCUDeviceCommunication::MCUDeviceCommunication() hardcodes the constants directly:
// ProtocolComm.cpp:74 — ignores ttyPort and baudRate globals entirely DeviceCommWithMCU(MCU_DEV_COMM_UART_DEVICE_NAME, MCU_DEV_COMM_UART_DEVICE_BAUD_RATE, ...) The -p and -b flags in the service ExecStart do nothing.
Bug: Iteration count doesn't equal full cycles
mcupep.cpp:43:
int i = readAttempts + 1; // = 6 with -c 5 while(i-- > 0) { ... state machine ... } One full send/receive cycle requires 4 state transitions (SEND → GET → PUB → DELAY). With i=6 the loop exits mid-cycle 2: SEND(1)→GET(2)→PUB(3)→DELAY(4)→SEND(5)→GET(6). The -c parameter doesn't configure the number of complete cycles.
Bug: TIMESTAMP_STATUS parses only 4 of 8 bytes, little-endian mismatch
ProtocolComm.cpp:279:
SymbolStatusData.TimestampStatus = StatusDataPacket[5]<<24 | StatusDataPacket[4]<<16 | StatusDataPacket[3]<<8 | StatusDataPacket[2]; MCU_DEV_TIMESTAMP_STATUS_DATA_BYTE_SIZE = 8, but only 4 bytes are read. The ReadBigEndian64 helper exists but isn't used here. Also setSystemTimeFromUnix() is commented out, so the timestamp is parsed but never applied.
Dead code: hardcoded ping variables
ProtocolComm.cpp:268-286 — ping = 0, the GetData() call is commented out, so the if(ping != 1) branch always executes and the else { RetStatus = true; } is unreachable.
ProtocolComm.cpp:425-436 — ping = 1 hardcoded, the else branch is unreachable.
Service has no ordering dependency
mcupep.service has no Before=mvcamera.service or After=initmodem.service. If the point is to initialize MCU before streaming starts, there's no guarantee of ordering.
Debug output going to stdout instead of glog
ProtocolComm.cpp is full of printf and std::cout calls. GLOG_log_dir=/var/log is set in the service environment but most output bypasses glog entirely. Also bool dbg = true is hardcoded in McuTest() at mcupep.cpp:59.
Minor: unused field + typo
MCUWriteState MCUWriteStateStatus declared in the class (ProtocolComm.hpp:455) but never read — a static local is used in SendDeviceStatusDataToMCU instead. ACCELEROMETER_SENSSITIVITY_STATUS — double 'S' in both the enum and switch. Makefile declares glib-2.0 and json-glib-1.0 as deps but neither is used.
Bug: Command-line args have no effect on UART
Conclusion: False AI trigger
Action: ttyPort/baudRate removed them from ProtocolComm.cpp
...but MCUDeviceCommunication::MCUDeviceCommunication() hardcodes the constants directly:
Analysis: MCUDeviceCommunication() constructor is initialized with default values
Action: Do nothing
Bug: Iteration count doesn't equal full cycles
int i = readAttempts + 1; // = 6 with -c 5 while(i-- > 0) { ... state machine ... } One full send/receive cycle requires 4 state transitions (SEND → GET → PUB → DELAY). With i=6 the loop exits mid-cycle 2: SEND(1)→GET(2)→PUB(3)→DELAY(4)→SEND(5)→GET(6). The -c parameter doesn't configure the number of complete cycles.
Analysis: For completeness testing, only one successful read cycle matters. The original is an infinite loop. We need a limited number of cycles. Action: I fixed this by making two successful cycles the default.
Bug: TIMESTAMP_STATUS parses only 4 of 8 bytes, little-endian mismatch
ProtocolComm.cpp:279:
SymbolStatusData.TimestampStatus = StatusDataPacket[5]<<24 | StatusDataPacket[4]<<16 | StatusDataPacket[3]<<8 | StatusDataPacket[2]; MCU_DEV_TIMESTAMP_STATUS_DATA_BYTE_SIZE = 8, but only 4 bytes are read. The ReadBigEndian64 helper exists but isn't used here. Also setSystemTimeFromUnix() is commented out, so the timestamp is parsed but never applied.
Anallysis: Good findinig
Action: Fixed.
Dead code: hardcoded ping variables
ProtocolComm.cpp:268-286 — ping = 0, the GetData() call is commented out, so the if(ping != 1) branch always executes and the else { RetStatus = true; } is unreachable.
ProtocolComm.cpp:425-436 — ping = 1 hardcoded, the else branch is unreachable.
Action: Extra code removed.
Service has no ordering dependency
mcupep.service has no Before=mvcamera.service or After=initmodem.service. If the point is to initialize MCU before streaming starts, there's no guarantee of ordering.
Analysis: The order doesn't matter. The main thing is that the service starts within the first 30-60 seconds.
Action: Do nothing.
Debug output going to stdout instead of glog
ProtocolComm.cpp is full of printf and std::cout calls. GLOG_log_dir=/var/log is set in the service environment but most output bypasses glog entirely. Also bool dbg = true is hardcoded in McuTest() at mcupep.cpp:59.
Analysis: bool dbg = true is for testing, it is not finished yet.
Action: Do nothing
Minor: unused field + typo
MCUWriteState MCUWriteStateStatus declared in the class (ProtocolComm.hpp:455) but never read — a static local is used in SendDeviceStatusDataToMCU instead. ACCELEROMETER_SENSSITIVITY_STATUS — double 'S' in both the enum and switch. Makefile declares glib-2.0 and json-glib-1.0 as deps but neither is used.
Action: glib-2.0 and json-glib-1.0 removed from Makefile