Skip to content

Commit 0a8b400

Browse files
c-jimenezc-jimenez
authored
Merge pull request #115 from c-jimenez/fix/metervalues_diagnostics
Fix maintenance, metervalues and triggermessage managers
2 parents 5273718 + da1fa37 commit 0a8b400

File tree

6 files changed

+223
-47
lines changed

6 files changed

+223
-47
lines changed

‎src/chargepoint/authent/AuthentLocalList.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ bool AuthentLocalList::handleMessage(const ocpp::messages::SendLocalListReq& req
124124
// Check local list activation
125125
if (m_ocpp_config.localAuthListEnabled())
126126
{
127-
if (request.listVersion > 0)
127+
if (request.listVersion >= 0)
128128
{
129129
// Check update list size
130130
if (request.localAuthorizationList.size() <= m_ocpp_config.sendLocalListMaxLength())

‎src/chargepoint/maintenance/MaintenanceManager.cpp‎

Lines changed: 51 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -586,12 +586,19 @@ void MaintenanceManager::processGetDiagnostics(std::string
586586
if (!success)
587587
{
588588
// Next retry
589-
nb_retries--;
590-
if (nb_retries != 0)
589+
if (nb_retries > 0)
591590
{
592-
LOG_WARNING << "GetDiagnostics : upload failed (" << nb_retries << " retrie(s) left - next retry in "
593-
<< retry_interval_s.count() << "s)";
594-
std::this_thread::sleep_for(retry_interval_s);
591+
nb_retries--;
592+
if (nb_retries != 0)
593+
{
594+
LOG_WARNING << "GetDiagnostics : upload failed (" << nb_retries << " retrie(s) left - next retry in "
595+
<< retry_interval_s.count() << "s)";
596+
std::this_thread::sleep_for(retry_interval_s);
597+
}
598+
else
599+
{
600+
LOG_WARNING << "GetDiagnostics : upload failed no retries left";
601+
}
595602
}
596603
}
597604

@@ -635,10 +642,12 @@ void MaintenanceManager::processUpdateFirmware(std::string
635642
ocpp::types::Optional<unsigned int> retry_interval,
636643
ocpp::types::DateTime retrieve_date)
637644
{
645+
646+
LOG_INFO << "UpdateFirmware : Waiting until retrieve date (" << retrieve_date.timestamp() << ") from now (" << DateTime::now() << ")";
647+
638648
// Check retrieve date
639649
if (retrieve_date > DateTime::now())
640650
{
641-
LOG_INFO << "UpdateFirmware : Waiting until retrieve date";
642651
std::this_thread::sleep_until(std::chrono::system_clock::from_time_t(retrieve_date.timestamp()));
643652
}
644653

@@ -667,12 +676,19 @@ void MaintenanceManager::processUpdateFirmware(std::string
667676
if (!success)
668677
{
669678
// Next retry
670-
nb_retries--;
671-
if (nb_retries != 0)
679+
if (nb_retries > 0)
672680
{
673-
LOG_WARNING << "FirmwareUpdate : download failed (" << nb_retries << " retrie(s) left - next retry in "
674-
<< retry_interval_s.count() << "s)";
675-
std::this_thread::sleep_for(retry_interval_s);
681+
nb_retries--;
682+
if (nb_retries != 0)
683+
{
684+
LOG_WARNING << "FirmwareUpdate : download failed (" << nb_retries << " retrie(s) left - next retry in "
685+
<< retry_interval_s.count() << "s)";
686+
std::this_thread::sleep_for(retry_interval_s);
687+
}
688+
else
689+
{
690+
LOG_WARNING << "FirmwareUpdate : download failed no retries left";
691+
}
676692
}
677693
}
678694

@@ -767,12 +783,19 @@ void MaintenanceManager::processGetLog(ocpp::types::LogEnumType type,
767783
if (!success)
768784
{
769785
// Next retry
770-
nb_retries--;
771-
if (nb_retries != 0)
786+
if (nb_retries > 0)
772787
{
773-
LOG_WARNING << "GetLog : upload failed (" << nb_retries << " retrie(s) left - next retry in " << retry_interval_s.count()
774-
<< "s)";
775-
std::this_thread::sleep_for(retry_interval_s);
788+
nb_retries--;
789+
if (nb_retries != 0)
790+
{
791+
LOG_WARNING << "GetLog : upload failed (" << nb_retries << " retrie(s) left - next retry in "
792+
<< retry_interval_s.count() << "s)";
793+
std::this_thread::sleep_for(retry_interval_s);
794+
}
795+
else
796+
{
797+
LOG_WARNING << "GetLog : upload failed retries left";
798+
}
776799
}
777800
}
778801

@@ -855,12 +878,19 @@ void MaintenanceManager::processSignedUpdateFirmware(std::string
855878
if (!success)
856879
{
857880
// Next retry
858-
nb_retries--;
859-
if (nb_retries != 0)
881+
if (nb_retries > 0)
860882
{
861-
LOG_WARNING << "SignedUpdateFirmware : download failed (" << nb_retries << " retrie(s) left - next retry in "
862-
<< retry_interval_s.count() << "s)";
863-
std::this_thread::sleep_for(retry_interval_s);
883+
nb_retries--;
884+
if (nb_retries != 0)
885+
{
886+
LOG_WARNING << "SignedUpdateFirmware : download failed (" << nb_retries << " retrie(s) left - next retry in "
887+
<< retry_interval_s.count() << "s)";
888+
std::this_thread::sleep_for(retry_interval_s);
889+
}
890+
else
891+
{
892+
LOG_WARNING << "SignedUpdateFirmware : download failed no retries left";
893+
}
864894
}
865895
}
866896

‎src/chargepoint/metervalues/MeterValuesManager.cpp‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -188,9 +188,20 @@ void MeterValuesManager::getTxStopMeterValues(unsigned int connector_id, std::ve
188188
bool MeterValuesManager::onTriggerMessage(ocpp::types::MessageTrigger message, const ocpp::types::Optional<unsigned int>& connector_id)
189189
{
190190
bool ret = false;
191-
if (connector_id.isSet() && (message == MessageTrigger::MeterValues))
191+
if (message == MessageTrigger::MeterValues)
192192
{
193-
processTriggered(connector_id);
193+
if (connector_id.isSet())
194+
{
195+
processTriggered(connector_id);
196+
}
197+
else
198+
{
199+
for (const Connector* connector : m_connectors.getConnectors())
200+
{
201+
unsigned int id = connector->id;
202+
processTriggered(id);
203+
}
204+
}
194205
ret = true;
195206
}
196207
return ret;
@@ -201,9 +212,20 @@ bool MeterValuesManager::onTriggerMessage(ocpp::types::MessageTriggerEnumType
201212
const ocpp::types::Optional<unsigned int>& connector_id)
202213
{
203214
bool ret = false;
204-
if (connector_id.isSet() && (message == MessageTriggerEnumType::MeterValues))
215+
if (message == MessageTriggerEnumType::MeterValues)
205216
{
206-
processTriggered(connector_id);
217+
if (connector_id.isSet())
218+
{
219+
processTriggered(connector_id);
220+
}
221+
else
222+
{
223+
for (const Connector* connector : m_connectors.getConnectors())
224+
{
225+
unsigned int id = connector->id;
226+
processTriggered(id);
227+
}
228+
}
207229
ret = true;
208230
}
209231
return ret;

‎src/chargepoint/trigger/TriggerMessageManager.cpp‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,14 @@ bool TriggerMessageManager::handleMessage(const ocpp::messages::TriggerMessageRe
8585
}
8686
else
8787
{
88-
// Check connector id
89-
if (!request.connectorId.isSet() || m_connectors.isValid(request.connectorId))
88+
// Check invalid connector id
89+
if (request.connectorId.isSet() && !m_connectors.isValid(request.connectorId))
90+
{
91+
error_code = ocpp::rpc::IRpc::RPC_ERROR_PROPERTY_CONSTRAINT_VIOLATION;
92+
error_message = "Invalid connector id";
93+
response.status = TriggerMessageStatus::Rejected;
94+
}
95+
else
9096
{
9197
// Call handler
9298
if (it->second->onTriggerMessage(request.requestedMessage, request.connectorId))
@@ -100,11 +106,6 @@ bool TriggerMessageManager::handleMessage(const ocpp::messages::TriggerMessageRe
100106
LOG_WARNING << "Trigger message rejected : " << trigger_message;
101107
}
102108
}
103-
else
104-
{
105-
error_code = ocpp::rpc::IRpc::RPC_ERROR_PROPERTY_CONSTRAINT_VIOLATION;
106-
error_message = "Invalid connector id";
107-
}
108109
}
109110

110111
return ret;
@@ -136,8 +137,14 @@ bool TriggerMessageManager::handleMessage(const ocpp::messages::ExtendedTriggerM
136137
}
137138
else
138139
{
139-
// Check connector id
140-
if (!request.connectorId.isSet() || m_connectors.isValid(request.connectorId))
140+
// Check invalid connector id
141+
if (request.connectorId.isSet() && !m_connectors.isValid(request.connectorId))
142+
{
143+
error_code = ocpp::rpc::IRpc::RPC_ERROR_PROPERTY_CONSTRAINT_VIOLATION;
144+
error_message = "Invalid connector id";
145+
response.status = TriggerMessageStatusEnumType::Rejected;
146+
}
147+
else
141148
{
142149
// Call handler
143150
if (it->second->onTriggerMessage(request.requestedMessage, request.connectorId))
@@ -151,11 +158,6 @@ bool TriggerMessageManager::handleMessage(const ocpp::messages::ExtendedTriggerM
151158
LOG_WARNING << "Extended trigger message rejected : " << trigger_message;
152159
}
153160
}
154-
else
155-
{
156-
error_code = ocpp::rpc::IRpc::RPC_ERROR_PROPERTY_CONSTRAINT_VIOLATION;
157-
error_message = "Invalid connector id";
158-
}
159161
}
160162

161163
return ret;

‎tests/chargepoint/metervalues/test_metervalues.cpp‎

Lines changed: 128 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,120 @@ static void checkClockAligned(const std::vector<std::pair<std::string, std::uniq
162162
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[2].value, "16");
163163
}
164164

165+
/** @brief Check all the sampled meter values */
166+
static void checkAllSampled(const std::vector<std::pair<std::string, std::unique_ptr<rapidjson::Document>>>& messages,
167+
ReadingContext context)
168+
{
169+
MeterValuesReq meter_value_req;
170+
171+
CHECK_EQ(messages.size(), 3u);
172+
CHECK_EQ(messages[0].first, METER_VALUES_ACTION);
173+
CHECK_EQ(messages[1].first, METER_VALUES_ACTION);
174+
CHECK_EQ(messages[2].first, METER_VALUES_ACTION);
175+
176+
CHECK(deserializeMeterValue((*messages[0].second), meter_value_req));
177+
CHECK_EQ(meter_value_req.connectorId, 0);
178+
CHECK_FALSE(meter_value_req.transactionId.isSet());
179+
CHECK_EQ(meter_value_req.meterValue.size(), 1u);
180+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue.size(), 4);
181+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[0].measurand, Measurand::CurrentImport);
182+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[0].phase, Phase::L1);
183+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[0].location, Location::Inlet);
184+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[0].format, ValueFormat::Raw);
185+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[0].unit, UnitOfMeasure::A);
186+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[0].context, context);
187+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[0].value, "10");
188+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[1].measurand, Measurand::CurrentImport);
189+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[1].phase, Phase::L2);
190+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[1].location, Location::Inlet);
191+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[1].format, ValueFormat::Raw);
192+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[1].unit, UnitOfMeasure::A);
193+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[1].context, context);
194+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[1].value, "20");
195+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[2].measurand, Measurand::CurrentImport);
196+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[2].phase, Phase::L3);
197+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[2].location, Location::Inlet);
198+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[2].format, ValueFormat::Raw);
199+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[2].unit, UnitOfMeasure::A);
200+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[2].context, context);
201+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[2].value, "30");
202+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[3].measurand, Measurand::EnergyActiveImportRegister);
203+
CHECK_FALSE(meter_value_req.meterValue[0].sampledValue[3].phase.isSet());
204+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[3].location, Location::Inlet);
205+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[3].format, ValueFormat::Raw);
206+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[3].unit, UnitOfMeasure::kWh);
207+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[3].context, context);
208+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[3].value, "123");
209+
210+
CHECK(deserializeMeterValue((*messages[1].second), meter_value_req));
211+
CHECK_EQ(meter_value_req.connectorId, 1);
212+
CHECK_FALSE(meter_value_req.transactionId.isSet());
213+
CHECK_EQ(meter_value_req.meterValue.size(), 1u);
214+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue.size(), 4);
215+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[0].measurand, Measurand::CurrentImport);
216+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[0].phase, Phase::L1);
217+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[0].location, Location::Outlet);
218+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[0].format, ValueFormat::Raw);
219+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[0].unit, UnitOfMeasure::A);
220+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[0].context, context);
221+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[0].value, "40");
222+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[1].measurand, Measurand::CurrentImport);
223+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[1].phase, Phase::L2);
224+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[1].location, Location::Outlet);
225+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[1].format, ValueFormat::Raw);
226+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[1].unit, UnitOfMeasure::A);
227+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[1].context, context);
228+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[1].value, "50");
229+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[2].measurand, Measurand::CurrentImport);
230+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[2].phase, Phase::L3);
231+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[2].location, Location::Outlet);
232+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[2].format, ValueFormat::Raw);
233+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[2].unit, UnitOfMeasure::A);
234+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[2].context, context);
235+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[2].value, "60");
236+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[3].measurand, Measurand::EnergyActiveImportRegister);
237+
CHECK_FALSE(meter_value_req.meterValue[0].sampledValue[3].phase.isSet());
238+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[3].location, Location::Outlet);
239+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[3].format, ValueFormat::Raw);
240+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[3].unit, UnitOfMeasure::kWh);
241+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[3].context, context);
242+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[3].value, "100");
243+
244+
CHECK(deserializeMeterValue((*messages[2].second), meter_value_req));
245+
CHECK_EQ(meter_value_req.connectorId, 2);
246+
CHECK_FALSE(meter_value_req.transactionId.isSet());
247+
CHECK_EQ(meter_value_req.meterValue.size(), 1u);
248+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue.size(), 4);
249+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[0].measurand, Measurand::CurrentImport);
250+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[0].phase, Phase::L1);
251+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[0].location, Location::Outlet);
252+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[0].format, ValueFormat::Raw);
253+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[0].unit, UnitOfMeasure::A);
254+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[0].context, context);
255+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[0].value, "70");
256+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[1].measurand, Measurand::CurrentImport);
257+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[1].phase, Phase::L2);
258+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[1].location, Location::Outlet);
259+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[1].format, ValueFormat::Raw);
260+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[1].unit, UnitOfMeasure::A);
261+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[1].context, context);
262+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[1].value, "80");
263+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[2].measurand, Measurand::CurrentImport);
264+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[2].phase, Phase::L3);
265+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[2].location, Location::Outlet);
266+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[2].format, ValueFormat::Raw);
267+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[2].unit, UnitOfMeasure::A);
268+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[2].context, context);
269+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[2].value, "90");
270+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[3].measurand, Measurand::EnergyActiveImportRegister);
271+
CHECK_FALSE(meter_value_req.meterValue[0].sampledValue[3].phase.isSet());
272+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[3].location, Location::Outlet);
273+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[3].format, ValueFormat::Raw);
274+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[3].unit, UnitOfMeasure::kWh);
275+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[3].context, context);
276+
CHECK_EQ(meter_value_req.meterValue[0].sampledValue[3].value, "23");
277+
}
278+
165279
/** @brief Check the sampled meter values */
166280
static void checkSampled(const std::vector<std::pair<std::string, std::unique_ptr<rapidjson::Document>>>& messages)
167281
{
@@ -825,18 +939,26 @@ TEST_SUITE("Metervalues component")
825939
rpc.clearCalls();
826940

827941
// Trigger meter values without connector id
828-
CHECK_FALSE(meter_mgr.onTriggerMessage(MessageTrigger::MeterValues, Optional<unsigned int>()));
829-
CHECK_FALSE(event_handler.methodCalled("getMeterValue", params));
942+
CHECK(meter_mgr.onTriggerMessage(MessageTrigger::MeterValues, Optional<unsigned int>()));
943+
CHECK(event_handler.methodCalled("getMeterValue", params));
830944

831945
// Check messages
832-
CHECK(rpc.getCalls().empty());
946+
checkAllSampled(rpc.getCalls(), ReadingContext::Trigger);
947+
948+
// Clear stubs
949+
event_handler.clearCalls();
950+
rpc.clearCalls();
833951

834952
// Extended trigger meter values without connector id
835-
CHECK_FALSE(meter_mgr.onTriggerMessage(MessageTriggerEnumType::MeterValues, Optional<unsigned int>()));
836-
CHECK_FALSE(event_handler.methodCalled("getMeterValue", params));
953+
CHECK(meter_mgr.onTriggerMessage(MessageTriggerEnumType::MeterValues, Optional<unsigned int>()));
954+
CHECK(event_handler.methodCalled("getMeterValue", params));
837955

838956
// Check messages
839-
CHECK(rpc.getCalls().empty());
957+
checkAllSampled(rpc.getCalls(), ReadingContext::Trigger);
958+
959+
// Clear stubs
960+
event_handler.clearCalls();
961+
rpc.clearCalls();
840962
}
841963

842964
TEST_CASE("Custom meter values")

‎tests/stubs/ChargePointEventsHandlerStub.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ bool ChargePointEventsHandlerStub::getLocalLimitationsSchedule(unsigned int
170170
/** @copydoc bool IChargePointEventsHandler::resetRequested(ocpp::types::ResetType) */
171171
bool ChargePointEventsHandlerStub::resetRequested(ocpp::types::ResetType reset_type)
172172
{
173-
m_calls["transactionDeAuthorized"] = {{"reset_type", ResetTypeHelper.toString(reset_type)}};
173+
m_calls["resetRequested"] = {{"reset_type", ResetTypeHelper.toString(reset_type)}};
174174
return callResult("resetRequested");
175175
}
176176

0 commit comments

Comments
 (0)