Skip to content

Commit e2ed6ad

Browse files
BobSaidHiBobSaidHi
committed
Ran PVS Studio Again, fixed some missing stuff / actual bugs
1 parent 284232d commit e2ed6ad

9 files changed

Lines changed: 91 additions & 57 deletions

File tree

‎include/INA260.hpp‎

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,19 @@ namespace INA260 {
1212
constexpr uint_fast16_t u_TO_m = 1000;
1313
constexpr uint_fast32_t u_TO_BASE = m_TO_BASE * u_TO_m;
1414
static constexpr int_fast8_t MIN_VOLTAGE_V = -32;
15-
static constexpr int_fast16_t MIN_VOLTAGE_mV = MIN_VOLTAGE_V * m_TO_BASE;
15+
static constexpr int_fast16_t MIN_VOLTAGE_mV =
16+
MIN_VOLTAGE_V * static_cast<int_fast16_t>(m_TO_BASE);
1617
static constexpr int_fast8_t MAX_VOLTAGE_V = 32;
1718
static constexpr int_fast16_t MAX_VOLTAGE_mV = MAX_VOLTAGE_V * m_TO_BASE;
1819
static constexpr int_fast8_t MIN_CURRENT_A = -15;
19-
static constexpr int_fast16_t MIN_CURRENT_mA = MIN_CURRENT_A * m_TO_BASE;
20+
static constexpr int_fast16_t MIN_CURRENT_mA =
21+
MIN_CURRENT_A * static_cast<int_fast16_t>(m_TO_BASE);
2022
static constexpr int_fast8_t MAX_CURRENT_A = 15;
2123
static constexpr int_fast16_t MAX_CURRENT_mA = MAX_CURRENT_A * m_TO_BASE;
2224
static constexpr int_fast16_t MIN_POWER_W =
2325
MIN_VOLTAGE_V * MIN_CURRENT_A * -1;
24-
static constexpr int_fast16_t MIN_POWER_mW = MIN_POWER_W * m_TO_BASE;
26+
static constexpr int_fast16_t MIN_POWER_mW =
27+
MIN_POWER_W * static_cast<int_fast16_t>(m_TO_BASE);
2528
static constexpr int_fast16_t MAX_POWER_W = MAX_VOLTAGE_V * MAX_CURRENT_A;
2629
static constexpr int_fast16_t MAX_POWER_mW = MAX_POWER_W * m_TO_BASE;
2730

@@ -65,18 +68,18 @@ namespace INA260 {
6568
unsigned long currentTime_us = micros();
6669
unsigned long dTime_us = currentTime_us - detail::lastUpdateTime_us;
6770

68-
auto vTemp_mV = (uint_fast32_t)powerSensor.readBusVoltage();
71+
auto vTemp_mV = static_cast<int_fast32_t>(powerSensor.readBusVoltage());
6972
if ((vTemp_mV > MAX_VOLTAGE_mV) ||
7073
(vTemp_mV < PSENSOR::MIN_VOLTAGE_mV)) {
7174
ESP_LOGE("TAG", "V reading out of bounds: %u mV", vTemp_mV);
72-
result &= false;
75+
result = false;
7376
} // Else: Valid reading
7477
prevVoltage_mV = voltage_mV;
7578
voltage_mV = vTemp_mV;
7679
int_fast16_t dVoltage_mV = voltage_mV - prevVoltage_mV;
7780
dVoltage_mVPS = dVoltage_mV * u_TO_BASE / dTime_us;
7881

79-
auto iTemp = (uint_fast32_t)powerSensor.readCurrent();
82+
auto iTemp = static_cast<int_fast32_t>(powerSensor.readCurrent());
8083
if ((iTemp > MAX_CURRENT_mA) || (iTemp < MIN_CURRENT_mA)) {
8184
ESP_LOGE("TAG", "C reading out of bounds: %d mA", iTemp);
8285
result &= false;
@@ -86,7 +89,7 @@ namespace INA260 {
8689
int_fast16_t dCurrent_mA = current_mA - prevCurrent_mA;
8790
dCurrent_mAPS = dCurrent_mA * u_TO_BASE / dTime_us;
8891

89-
auto pTemp = (uint_fast32_t)powerSensor.readPower();
92+
auto pTemp = static_cast<int_fast32_t>(powerSensor.readPower());
9093
if ((pTemp > (MAX_VOLTAGE_mV * MAX_CURRENT_A)) ||
9194
(pTemp < (PSENSOR::MIN_VOLTAGE_mV * MIN_CURRENT_A))) {
9295
ESP_LOGE("TAG", "P out of bounds: %d mW", pTemp);
@@ -109,31 +112,31 @@ namespace INA260 {
109112
*/
110113
etl::string<LOG_STRING_SIZE> getLogString() {
111114
etl::string<LOG_STRING_SIZE> logString(TAG); // 3 chars
112-
logString.append(": mV: "); // 6 chars
115+
(void)logString.append(": mV: "); // 6 chars
113116

114117
etl::format_spec decFormatA;
115-
decFormatA.width(5).fill('0'); // [5 chars]
118+
(void)decFormatA.width(5).fill('0'); // [5 chars]
116119
/**
117120
* @details I don't think we need strong guarantees on logging data
118121
* @see
119122
* https://stackoverflow.com/questions/12346487/what-do-each-memory-order-mean
120123
* @see https://en.cppreference.com/cpp/atomic/memory_order
121124
*/
122125
etl::to_string(voltage_mV.load(), logString, decFormatA,
123-
true); // 5 chars
124-
logString.append(", mA: "); // 6 chars
126+
true); // 5 chars
127+
(void)logString.append(", mA: "); // 6 chars
125128
etl::to_string(current_mA.load(), logString, decFormatA,
126-
true); // 5 chars
127-
logString.append(", mW: "); // 6 chars
129+
true); // 5 chars
130+
(void)logString.append(", mW: "); // 6 chars
128131
etl::to_string(power_mW.load() / m_TO_BASE, logString, decFormatA,
129-
true); // 5 chars
130-
logString.append(", mV/s: "); // 8 chars
132+
true); // 5 chars
133+
(void)logString.append(", mV/s: "); // 8 chars
131134
etl::to_string(dVoltage_mVPS.load(), logString, decFormatA,
132-
true); // 5 chars
133-
logString.append(", mA/s: "); // 8 chars
135+
true); // 5 chars
136+
(void)logString.append(", mA/s: "); // 8 chars
134137
etl::to_string(dCurrent_mAPS.load(), logString, decFormatA,
135-
true); // 5 chars
136-
logString.append(", mW/s: "); // 8 chars
138+
true); // 5 chars
139+
(void)logString.append(", mW/s: "); // 8 chars
137140
etl::to_string(dPower_mWPS.load(), logString, decFormatA,
138141
true); // 5 chars
139142

‎include/LoadConfig.hpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,9 @@ namespace PSENSOR {
128128
constexpr int_fast16_t MAX_VOLTAGE_V = 48; // CONFIG
129129
// CONFIG
130130
constexpr uint_fast16_t MAX_VOLTAGE_mV = MAX_VOLTAGE_V * m_TO_BASE;
131-
constexpr int_fast16_t MIN_CURRENT_A = -20; // CONFIG
132-
constexpr int_fast16_t MIN_CURRENT_mA = MIN_CURRENT_A * m_TO_BASE; // CONFIG
131+
constexpr int_fast16_t MIN_CURRENT_A = -20; // CONFIG
132+
constexpr int_fast16_t MIN_CURRENT_mA =
133+
MIN_CURRENT_A * static_cast<int_fast16_t>(m_TO_BASE); // CONFIG
133134
constexpr int_fast16_t MAX_CURRENT_A = 20; // CONFIG
134135
constexpr int_fast16_t MAX_CURRENT_mA = MAX_CURRENT_A * m_TO_BASE; // CONFIG
135136
// CONFIG

‎include/LoadContainer.hpp‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,21 +50,29 @@ class LoadContainer {
5050
inline bool isSteadyRPM() const {
5151
return (angularAccell_RPMPS < 20);
5252
} // todo - steady power is actually more important
53-
inline bool isTargetRPMExceeded() const { return false; } // todo
53+
inline bool isTargetRPMExceeded() const {
54+
constexpr uint_fast16_t TARGET_RPM = 2200; // todo
55+
return (currentRPM > TARGET_RPM);
56+
} // todo
5457

5558
inline void updateSafetyFlag(bool safetyFlag) {
5659
if (digitalRead(UM_PROS3::ESTOP_PIN) == LOW) {
5760
this->safetyFlag = ESTOP_TYPE_FAST::BUTTON;
5861
} else if (abs(INA260::current_mA) <
5962
PSENSOR::LOAD_SHED_I_THRESHOLD_mA) {
63+
this->safetyFlag = ESTOP_TYPE_FAST::LOAD_DISCONNECT;
64+
} else {
6065
this->safetyFlag = ESTOP_TYPE_FAST::NONE;
6166
}
6267
}
6368
// inline void updatePowerPositive(bool powerPositive) {
6469
// this->powerPositive = powerPositive;
6570
// }
6671
inline void setLoadGPIO(uint_fast8_t value) {
67-
loadDevice.setGPIO((uint32_t)value);
72+
bool result = loadDevice.setGPIO(static_cast<uint32_t>(value));
73+
if (!result) {
74+
ESP_LOGE(TAG, "Failed to set load GPIO");
75+
}
6876
}
6977
inline void setRPM(int_fast16_t rpm) { this->currentRPM = rpm; }
7078
inline int_fast16_t getRPM() const { return this->currentRPM; }

‎include/LoadFSM.hpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class LoadFSM {
4848
* @details Trim -1 to an 8-bit unsigned integer(255), unsigned extend
4949
* to uint_fast8_t
5050
*/
51-
ERROR = (uint_fast8_t)(uint8_t)-1
51+
ERROR = static_cast<uint_fast8_t>(static_cast<uint8_t>(-1))
5252
};
5353

5454
/**
@@ -72,7 +72,9 @@ class LoadFSM {
7272
(load.getSafetyFlag() != ESTOP_TYPE_FAST::NONE)) {
7373
// sESTOP -> sESTOP: Nothing to do
7474
return UPDATE_RESULT::NO_CHANGE;
75-
} // else: ~safetyTask
75+
} else {
76+
// else: ~safetyTask
77+
}
7678

7779
// Check reset conditions
7880
constexpr uint_fast16_t START_RUN_2_RPM = 500; // todo

‎include/LoadTasks.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88

99
// MARK: Datatypes & Constants
1010
struct TaskInfo {
11-
const TaskFunction_t function;
12-
const char *const name;
11+
const TaskFunction_t function = nullptr;
12+
const char *const name = nullptr;
1313
const configSTACK_DEPTH_TYPE stackSize_bytes = 1024;
1414
void *const pvParameters = nullptr;
1515
const UBaseType_t priority; // Note: Task priority must be <25 for some

‎lib/LoadComms/LoadComms.cpp‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ bool LoadComms::begin() {
5353
}
5454

5555
etl::array<uint8_t, 6> MACAddress = {0};
56+
// TODO: Conversions between pointers and integer types should not be
57+
// performed. Consider inspecting the second argument of the
58+
// 'esp_wifi_get_mac' function call.
5659
esp_err_t opStatus = esp_wifi_get_mac(WIFI_IF_STA, MACAddress.data());
5760
if (opStatus != ESP_OK) {
5861
ESP_LOGE(TAG, "Failed to get MAC address: %d", opStatus);
@@ -99,7 +102,7 @@ bool LoadComms::begin() {
99102

100103
esp_err_t LoadComms::setupPeer_() {
101104
esp_now_peer_info_t peerInfo = {};
102-
memcpy(peerInfo.peer_addr, NACELLE_MAC, 6);
105+
(void)memcpy(peerInfo.peer_addr, NACELLE_MAC, 6);
103106
peerInfo.channel = 0;
104107
peerInfo.encrypt = false;
105108

@@ -122,7 +125,7 @@ etl::string<LoadComms::LOG_STRING_SIZE> LoadComms::getLogString() const {
122125
logString.append(": TxE: "); // 7 chars
123126

124127
etl::format_spec decFormatA;
125-
decFormatA.width(6).fill(
128+
(void)decFormatA.width(6).fill(
126129
'0'); // [6 chars, expecting up to 30 mins * (1/(2ms)) = 900,000 events]
127130
/**
128131
* @details I don't think we need strong guarantees on logging data
@@ -135,17 +138,17 @@ etl::string<LoadComms::LOG_STRING_SIZE> LoadComms::getLogString() const {
135138
logString.append(", TxBS: "); // 8 chars
136139

137140
etl::format_spec decFormatB;
138-
decFormatB.width(7).fill('0'); // [7 chars]
141+
(void)decFormatB.width(7).fill('0'); // [7 chars]
139142
etl::to_string(bytesSent.load(std::memory_order_relaxed), logString,
140-
decFormatB, true); // 7 chars
141-
logString.append(", TxBF: "); // 8 chars
143+
decFormatB, true); // 7 chars
144+
(void)logString.append(", TxBF: "); // 8 chars
142145
etl::to_string(bytesNotSent.load(std::memory_order_relaxed), logString,
143146
decFormatB, true); // 7 chars
144147

145-
logString.append(", RxE: "); // 7 chars
148+
(void)logString.append(", RxE: "); // 7 chars
146149
etl::to_string(rxEvents.load(std::memory_order_relaxed), logString,
147-
decFormatA, true); // 6 chars
148-
logString.append(", RxB: "); // 7 chars
150+
decFormatA, true); // 6 chars
151+
(void)logString.append(", RxB: "); // 7 chars
149152
etl::to_string(bytesReceived.load(std::memory_order_relaxed), logString,
150153
decFormatB, true); // 7 chars
151154

@@ -206,8 +209,9 @@ bool LoadComms::sendLoadboxData(int16_t d_mVPS, int16_t current_mA,
206209
int16_t dIPS, ESTOP_TYPE_NET safety) {
207210
// if (now - lastSendTime_ >= LOAD_COMMS_SEND_PERIOD_MS) {
208211
makeLoadboxPacket(outgoingPacket_, d_mVPS, current_mA, dIPS, safety);
209-
esp_err_t result = esp_now_send(NACELLE_MAC, (uint8_t *)&outgoingPacket_,
210-
sizeof(outgoingPacket_));
212+
esp_err_t result =
213+
esp_now_send(NACELLE_MAC, reinterpret_cast<uint8_t *>(&outgoingPacket_),
214+
sizeof(outgoingPacket_));
211215
if (result == ESP_OK) {
212216
lastSendTime_ = millis();
213217
linkAlive_ = true;

‎lib/LoadComms/LoadComms.hpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,10 @@ class LoadComms {
110110
LogData getLogData() const;
111111

112112
private:
113-
NacellePacket incomingPacket_; ///< Received packet from nacelle.
114-
LoadboxPacket outgoingPacket_; ///< Outgoing packet to send.
115-
unsigned long lastSendTime_; ///< Timestamp of last transmission.
116-
unsigned long lastRxTime_; ///< Timestamp of last received packet.
113+
NacellePacket incomingPacket_ = {0}; ///< Received packet from nacelle.
114+
LoadboxPacket outgoingPacket_ = {0}; ///< Outgoing packet to send.
115+
unsigned long lastSendTime_; ///< Timestamp of last transmission.
116+
unsigned long lastRxTime_; ///< Timestamp of last received packet.
117117
static std::atomic<uint_fast32_t>
118118
txEvents; // DONE: check against last years code
119119
static std::atomic<uint_fast32_t> bytesSent;

‎lib/MCP23008T/MCP23008T.hpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -267,14 +267,14 @@ class MCP23008T {
267267
*/
268268
etl::string<LOG_STRING_SIZE> getLogString() {
269269
etl::string<LOG_STRING_SIZE> logString(TAG); // 3 chars
270-
logString.append(": @ox"); // 5 chars
270+
(void)logString.append(": @ox"); // 5 chars
271271

272272
etl::format_spec format2;
273-
format2.hex().width(2).fill('0'); // [2 chars]
273+
(void)format2.hex().width(2).fill('0'); // [2 chars]
274274
etl::to_string(getAddress(), logString, format2, true); // 2 chars
275275

276276
etl::format_spec format3;
277-
format3.hex().width(3).fill('0'); // [3 chars]
277+
(void)format3.hex().width(3).fill('0'); // [3 chars]
278278
etl::to_string(readIntCap(), logString, format3, true); // 3 chars
279279
etl::to_string(readIntFlag(), logString, format3, true); // 3 chars
280280

‎src/main.cpp‎

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,13 @@ void setup() {
260260
continue;
261261
}
262262

263+
/**
264+
* @details PVS-Studio: "The modulo by 1 operation is meaningless.
265+
* The result will always be zero."
266+
* @see https://pvs-studio.com/en/docs/warnings/v1063/
267+
* @details PVS=Studio: "Expression is always false."
268+
* @see https://pvs-studio.com/en/docs/warnings/v547/
269+
*/
263270
if (taskDesc->stackSize_bytes % sizeof(StackType_t) != 0) {
264271
ESP_LOGE(TAG, "Stack size not aligned");
265272
continue;
@@ -295,16 +302,20 @@ void setup() {
295302
// 0.0f, 1500.0f); // todo - just a quick performances test
296303

297304
// Configure FSM
305+
// PVS-Studio: One of these is always true...
298306
LoadFSM::UPDATE_RESULT result = loadFSM.updateState();
299307
if (result == LoadFSM::UPDATE_RESULT::ERROR) {
300-
ESP_LOGE(TAG, "Error during FSM init., %d", (uint_fast8_t)result);
308+
// PVS=Studio: "Expression [...] is always false."
309+
ESP_LOGE(TAG, "Error during FSM init., %d",
310+
static_cast<uint_fast8_t>(result));
301311
} else if (result == LoadFSM::UPDATE_RESULT::STATE_CHANGED) {
302312
ESP_LOGI(TAG, "Initialized FSM to state %d",
303-
(uint_fast8_t)loadFSM.getCurrentState());
313+
static_cast<uint_fast8_t>(loadFSM.getCurrentState()));
304314
} else if (result == LoadFSM::UPDATE_RESULT::NO_CHANGE) {
305315
ESP_LOGE(TAG, "Failed to enter a valid state");
306316
} else {
307-
ESP_LOGE(TAG, "Unknown FSM init. result: %d", (uint_fast8_t)result);
317+
ESP_LOGE(TAG, "Unknown FSM init. result: %d",
318+
static_cast<uint_fast8_t>(result));
308319
}
309320

310321
ESP_LOGI(TAG, "Setup complete!");
@@ -324,13 +335,15 @@ void setup() {
324335
[[noreturn]] void
325336
vTaskUpdateFSM([[maybe_unused]] void *pvParameters) { // NOSONAR
326337
while (true) {
327-
// LoadFSM::UPDATE_RESULT result = loadFSM.updateState();
328-
// if (result == LoadFSM::UPDATE_RESULT::STATE_CHANGED) {
329-
// ESP_LOGI(TAG, "FSM State Changed: %d",
330-
// loadFSM.getCurrentState());
331-
// } else if (result == LoadFSM::UPDATE_RESULT::ERROR) {
332-
// ESP_LOGE(TAG, "Error updating FSM state");
333-
// }
338+
LoadFSM::UPDATE_RESULT result = loadFSM.updateState();
339+
if (result == LoadFSM::UPDATE_RESULT::STATE_CHANGED) {
340+
ESP_LOGI(TAG, "FSM State Changed: %d", loadFSM.getCurrentState());
341+
} else if (result == LoadFSM::UPDATE_RESULT::ERROR) {
342+
// PVS-Studio says this is always false
343+
ESP_LOGE(TAG, "Error updating FSM state");
344+
} else {
345+
// No change, nothing to log
346+
}
334347
delay(RUN::TASK_INTERVALS::TI_FSM_mS);
335348
}
336349
}
@@ -626,7 +639,7 @@ constexpr uint32_t LOG_ITEM_INTERVAL_MS = RUN::TASK_INTERVALS::TI_LOG_DATA_ms;
626639
float tsens_out;
627640
ESP_ERROR_CHECK(
628641
temperature_sensor_get_celsius(tempSensHandle, &tsens_out));
629-
auto tempTrunc_C = (int32_t)tsens_out;
642+
auto tempTrunc_C = static_cast<int32_t>(tsens_out);
630643
constexpr int32_t MAX_EXT_TEMP = 105;
631644
constexpr int32_t MIN_EXT_TEMP = -40;
632645
if (tempTrunc_C > MAX_EXT_TEMP || tempTrunc_C < MIN_EXT_TEMP) {
@@ -638,6 +651,11 @@ constexpr uint32_t LOG_ITEM_INTERVAL_MS = RUN::TASK_INTERVALS::TI_LOG_DATA_ms;
638651
ESP_ERROR_CHECK(temperature_sensor_disable(tempSensHandle));
639652
// delay(LOG_ITEM_INTERVAL_MS);
640653

654+
ESP_LOGI(TAG, "Current State: %d", loadFSM.getCurrentState());
655+
delay(LOG_ITEM_INTERVAL_MS);
656+
657+
// TODO: Improve logging, check ESTOP logic
658+
641659
ESP_LOGI(TAG, "%s", INA260::getLogString().c_str());
642660

643661
static unsigned int prevTime_us = 0;
@@ -669,8 +687,6 @@ constexpr uint32_t LOG_ITEM_INTERVAL_MS = RUN::TASK_INTERVALS::TI_LOG_DATA_ms;
669687
prevTime_us = currentTime_us;
670688
lastLogData = currentLogData;
671689

672-
prevTime_us = currentTime_us;
673-
674690
delay(LOG_ITEM_INTERVAL_MS);
675691

676692
// esp_wifi_get_bandwidth

0 commit comments

Comments
 (0)