-
Notifications
You must be signed in to change notification settings - Fork 53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
B07 get base report simplifications #397
Conversation
Signed-off-by: Soumya Subramanya <[email protected]>
Signed-off-by: Soumya Subramanya <[email protected]>
c8bed6e
to
ae8fcc8
Compare
Signed-off-by: Soumya Subramanya <[email protected]> fix formatting Signed-off-by: Soumya Subramanya <[email protected]> fix formatting Signed-off-by: Soumya Subramanya <[email protected]> fix more formatting Signed-off-by: Soumya Subramanya <[email protected]>
1a2581f
to
96c1237
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small suggestions
include/ocpp/v201/device_model.hpp
Outdated
@@ -222,8 +209,8 @@ class DeviceModel { | |||
/// \param component_criteria | |||
/// \return | |||
std::vector<ReportData> | |||
get_custom_report_data(const std::optional<std::vector<ComponentVariable>>& component_variables = std::nullopt, | |||
const std::optional<std::vector<ComponentCriterionEnum>>& component_criteria = std::nullopt); | |||
get_report_data(const std::optional<std::vector<ComponentVariable>>& component_variables = std::nullopt, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I preferred the get_custom_report_data
name since it really makes clear what this function is for too.
include/ocpp/v201/device_model.hpp
Outdated
get_report_data(const std::optional<ReportBaseEnum>& report_base = std::nullopt, | ||
const std::optional<std::vector<ComponentVariable>>& component_variables = std::nullopt, | ||
const std::optional<std::vector<ComponentCriterionEnum>>& component_criteria = std::nullopt); | ||
std::vector<ReportData> get_base_report_data(const std::optional<ReportBaseEnum>& report_base = std::nullopt); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this still need to be an optional? I assume it is now an always required variable here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i guess not
Signed-off-by: Soumya Subramanya <[email protected]>
Signed-off-by: Soumya Subramanya <[email protected]>
some basic refactoring to remove redundant code from get base report