Add clang-tidy config and associated fixes #1146

hikinggrass · 2025-10-13T12:11:42Z

Describe your changes

Issue ticket number and link

Checklist before requesting a review

I have performed a self-review of my code
I have made corresponding changes to the documentation
If OCPP 2.0.1 or OCPP2.1: I have updated the OCPP 2.x status document
I read the contribution documentation and made sure that my changes meet its requirements

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Prefer .empty() to .size() == 0 in generated messages Remove include that is not needed from 1.6 ocpp types Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

…) == 0 Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Keeping the behavior the same, but the algorithm looks suspicious Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

mlitre

I've reviewed until include/ocpp/v16/charge_point_configuration.hpp excluded.

mlitre · 2025-10-28T13:45:58Z

include/ocpp/common/aligned_timer.hpp

 private:
    system_time_point start_point;
-    std::chrono::seconds call_interval;
+    std::chrono::seconds call_interval = std::chrono::seconds(0);


I am guessing the behavior is that the timer doesn't start when the interval is 0, I just want to confirm though

mlitre · 2025-10-28T13:47:22Z

include/ocpp/common/message_queue.hpp

-    int transaction_message_attempts;
-    int transaction_message_retry_interval; // seconds
+    int transaction_message_attempts = 0;
+    int transaction_message_retry_interval = 0; // seconds


Same here, is 0 a correct default?

mlitre · 2025-10-28T13:47:53Z

include/ocpp/common/message_queue.hpp

    // threshold for the accumulated sizes of the queues; if the queues exceed this limit,
    // messages are potentially dropped in accordance with OCPP 2.0.1. Specification (cf. QueueAllMessages parameter)
-    int queues_total_size_threshold;
+    int queues_total_size_threshold = 0;


Maybe we should increase this default

mlitre · 2025-10-28T14:36:33Z

include/ocpp/common/types.hpp


 struct StateOfCharge {
-    float value;                         ///< State of Charge in percent
+    float value = 0;                     ///< State of Charge in percent


Wondering as well if these are the correct defaults to have here?

mlitre · 2025-11-12T10:34:00Z

3rd_party/websocketpp_utils/uri.hpp

+static const uint16_t uri_default_port = 80;
 /// Default port for wss://
-static uint16_t const uri_default_secure_port = 443;
+static const uint16_t uri_default_secure_port = 443;


Jumping on something I saw, maybe we should use std::uint16_t?

mlitre · 2025-11-12T10:38:36Z

lib/ocpp/v2/functional_blocks/remote_transaction_control.cpp

+namespace {
+///
+/// \brief Check if one of the connectors of the evse is available (both connectors faulted or unavailable or on of
+///        the connectors occupied).
+/// \param evse Evse to check.
+/// \return True if at least one connector is not faulted or unavailable.
+///
+bool is_evse_connector_available(EvseInterface& evse);
+} // namespace


Why not just put the implementation directly here?

mlitre · 2025-11-12T10:40:57Z

lib/ocpp/v2/functional_blocks/smart_charging.cpp

+/// \brief validates that the given \p profile from a RequestStartTransactionRequest is of the correct type
+/// TxProfile
+ProfileValidationResultEnum validate_request_start_transaction_profile(const ChargingProfile& profile);
+
+/// \brief sets attributes of the given \p charging_schedule_period according to the specification.
+/// 2.11. ChargingSchedulePeriodType if absent numberPhases set to 3
+void conform_schedule_number_phases(int32_t profile_id, ChargingSchedulePeriod& charging_schedule_period);
+
+///
+/// \brief sets attributes of the given \p profile according to the specification.
+/// 2.10. ChargingProfileType validFrom if absent set to current date
+/// 2.10. ChargingProfileType validTo if absent set to max date
+///
+void conform_validity_periods(ChargingProfile& profile);


Why not just move the definitions here directly?

mlitre · 2025-11-12T10:43:34Z

lib/ocpp/v2/average_meter_values.cpp

 namespace v2 {
-AverageMeterValues::AverageMeterValues() {
+namespace {
+bool is_avg_meas(const SampledValue& sample);


mlitre · 2025-11-12T10:55:07Z

lib/ocpp/common/websocket/websocket.cpp


-Websocket::~Websocket() {
-}
+Websocket::~Websocket() = default;


Let's just define default in the header file

mlitre · 2025-11-12T14:26:57Z

lib/ocpp/v2/evse.cpp


    for (uint32_t i = 1; i <= number_of_connectors; ++i) {
-        Connector* connector;
+        const Connector* connector = nullptr;


Why is this const, we set the value two lines below

mlitre · 2025-11-12T14:28:27Z

lib/ocpp/v2/evse_manager.cpp


 bool EvseManager::does_connector_exist(const int32_t evse_id, const CiString<20> connector_type) const {
-    const EvseInterface* evse;
+    const EvseInterface* evse = nullptr;


We should probably set this to not const

mlitre · 2025-11-12T14:36:09Z

lib/ocpp/v2/monitoring_updater.cpp

+    try {
+        stop_monitoring();
+    } catch (...) {
+        return;


Let's add a log

mlitre · 2025-11-12T14:39:36Z

lib/ocpp/v21/functional_blocks/bidirectional.cpp


-ocpp::v2::Bidirectional::~Bidirectional() {
-}
+ocpp::v2::Bidirectional::~Bidirectional() = default;


Let's move this to the header

mlitre · 2025-11-12T14:44:36Z

src/code_generator/common/templates/ocpp_types.hpp.jinja

-{% if namespace == "v16" %}
-    #include <ocpp/v16/types.hpp>
-{% endif %}


Isn't this still required?

hikinggrass added 30 commits October 13, 2025 13:17

Add clang-tidy config

e27213e

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Fix clang-format config to always keep qualifiers aligned left

bd0fc78

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Update gitignore file

dc9d31a

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Fix doxygen comment formatting on generated files

2612ee1

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Fix header guard

84bec6d

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Use const refs in generated messages and types

c6716b3

Prefer .empty() to .size() == 0 in generated messages Remove include that is not needed from 1.6 ocpp types Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Use const (ref) where possible

ade4503

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Use explicit comparisons in if statements and prefer empty() to size(…

0da2471

…) == 0 Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Use = default for ctors and use override for virtual functions

5abbbfc

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Use ctor initializer list

af0de49

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Do not declare multiple variables at once

4514ecf

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Initialize members and variables

a089d90

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Use using instead of typedef

9a39eb0

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Simplify if/else handling, eg. no use of else after return

e6d200e

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Use stoul instead of atoi

2578082

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Fix missing value in enum declaration

c162d3e

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Add clamp_to and convert_to_positive_size_t utils functions

41d1032

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Use anonymous namespace instead of static

6d12cfd

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Use enum class instead of enum

1a8909c

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

remove repeating visibilities

f870412

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Fix parameter names not being identical in hpp and cpp

8eb99ea

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

remove redundant inline

e855c0c

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Access static member via class not this

1f1f748

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Use move

6d0606a

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Make log_on_fail static

0e90f07

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Fix size_t usage in profile

102cd08

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Fix includes

7e2797b

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Use std::is_same_v

e00d560

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Add readability NOLINT comments

6d46530

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Reserve space in containers

d69c5da

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

hikinggrass added 22 commits October 13, 2025 13:17

Ensure that reset() of object instead of smart ptr is used

755223d

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Fix unchecked optional access

20b7176

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Add some utils tests

48d2b0f

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Modify drop_transaction_data algorithm to fix some clang-tidy issues

b21fbee

Keeping the behavior the same, but the algorithm looks suspicious Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Ensure operator precedence

a5ebe13

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Use iterator instead of tracking a position manually

08c5fd0

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Comment out unused parameters

6388e90

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

catch exceptions in destructors

0a22df2

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Remove moves that do not do anything

b8e3b07

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Add NOLINT comments for needed reinterpret_casts

0808f2e

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

clamp values so they do not overflow

ebda6de

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Use lambdas instead of bind

d639af2

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Use emplace_back instead of push_back

3439d7e

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Cast values to appropriate types

b48955b

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Remove unused variables and initialization

1a1af5b

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

compare to string::npos

b748dc8

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Do not return void from void function

feeefe1

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Remove redundant .get() or .c_str()

bfab6ea

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

remove duplicate if statement

44e22c4

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

use container.find()

e33ce88

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Ignore widening conversion warnings on very small constant values

e1975ae

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Fix type of variable

c76826d

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

hikinggrass requested review from Pietfried, marcemmers and mlitre as code owners October 13, 2025 12:11

clang-format

8c80858

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Pietfried assigned mlitre and Pietfried Oct 16, 2025

mlitre reviewed Oct 28, 2025

View reviewed changes

mlitre reviewed Nov 12, 2025

View reviewed changes

Add clang-tidy config and associated fixes #1146

Are you sure you want to change the base?

Add clang-tidy config and associated fixes #1146

Uh oh!

Conversation

hikinggrass commented Oct 13, 2025

Describe your changes

Issue ticket number and link

Checklist before requesting a review

Uh oh!

mlitre left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants