Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Feature/376 ocpp 201 california pricing requirements tariff and cost #707

Merged
merged 30 commits into from
Aug 13, 2024
Merged

Feature/376 ocpp 201 california pricing requirements tariff and cost #707

merged 30 commits into from
Aug 13, 2024

Conversation

maaikez
Copy link
Contributor

@maaikez maaikez commented Jul 23, 2024 •
edited
Loading

Describe your changes

Implement california pricing for 1.6.

Issue ticket number and link

#376

Checklist before requesting a review

maaikez added 15 commits July 8, 2024 18:29
@maaikez
Start implementing California Pricing for 1.6: add type structs for c…
f253e03 
…allbacks in 'common' so they can be used for both 1.6 and 2.01.

Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@maaikez
Implement some from / to json functions for running cost
0077745 
Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@maaikez
Define structs and create conversions for RunningCost and DisplayMess…
701a0ff 
…age.

Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@maaikez
Write some conversions.
d56f2f4 
Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@maaikez
Start implementing California Pricing for OCPP 1.6
168610c 
Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@maaikez
Send session id instead of transaction id (convert!). Add some json c…
7b2df00 
…onfig schemas.

Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@maaikez
Fix some crashes.
936cb6d 
Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@maaikez
Fix some conversions to / from json. Add configuration items needed f…
0ddc250 
…or CostAndPrice and multi language.

Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@maaikez
Add number of decimals to config. Start with 201 implementation.
a5936f2 
Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@maaikez
Implement todo's or remove them. Start implementing triggers.
2756d8f 
Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@maaikez
Implement trigger timer for sending metervalues.
3644320 
Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@maaikez
Remove 2.0.1 stuff.
30deec6 
Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@maaikez
Change DefaultPrice configuration item and other configurations. Add …
4617c42 
…get / set functions for configurations.

Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@maaikez
Start time offset timer so the time offset will be set at the correct…
ec2d1c2 
… transition date / time.

Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@maaikez
Merge branch 'main' into feature/376-ocpp-201-california-pricing-requ…
b99a53a 
…irements-tariff-and-cost

Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@maaikez maaikez linked an issue Jul 23, 2024 that may be closed by this pull request
OCPP 2.0.1 California Pricing Requirements: Tariff and Cost #376
Closed
@maaikez
Implement DefaultPriceText languages.
0d21f65 
Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@maaikez
Some fixes in setting the default price text. Set cost and price conf…
2ce9081 
…iguration items in user config.

Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@maaikez
Trigger metervalue on status is an array, not a single status (can be…
1927cbf 
… multiple statusses).

Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@maaikez
Get transaction from transaction id / tag id
be630d0 
Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@maaikez
clangformat
fee912b 
Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
Pietfried
Pietfried requested changes Jul 29, 2024
View reviewed changes
Copy link
Contributor

@Pietfried Pietfried left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See in line comments.

The handle_set_user_price and handle_set_session_cost became quite large and I think there is some room to restructure them and improve the readibility (some in line comments already address this)

config/v16/profile_schemas/CostAndPrice.json Outdated Show resolved Hide resolved
config/v16/profile_schemas/CostAndPrice.json Show resolved Hide resolved
include/ocpp/common/types.hpp Outdated Show resolved Hide resolved
include/ocpp/common/utils.hpp Show resolved Hide resolved
include/ocpp/v16/charge_point.hpp Outdated Show resolved Hide resolved
lib/ocpp/v16/charge_point_impl.cpp Outdated Show resolved Hide resolved
lib/ocpp/v16/charge_point_impl.cpp Outdated Show resolved Hide resolved
lib/ocpp/v16/charge_point_impl.cpp Show resolved Hide resolved
lib/ocpp/v16/charge_point_impl.cpp

json data;
try {
data = json(msg.value());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of operating on the raw json I think you could also define a SetUserPrice type like you have done it for RunningCost

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'SetUserPrice' translates into display messages here. Because it is in the end a message that should be displayed for a specific transaction. So do you suggest a new type instead of DisplayMessage or a conversion function with the json as input and a vector of display messages as output?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean the EVerest DisplayMessage type? This should not be of interest in libocpp. My idea was to define a struct thats initialized by parsing the json instead of operating on the raw json

lib/ocpp/v16/charge_point_impl.cpp Outdated Show resolved Hide resolved
@maaikez
Review comments. Add documentation, make code a bit more readable, fi…
94eb725 
…x some issues. When session id is not found, return rejected and not send pricing messages.

Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@maaikez
Make code more readable hopefully by moving code to functions.
c014699 
Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@Pietfried Pietfried mentioned this pull request Jul 31, 2024
Closed
@maaikez maaikez marked this pull request as ready for review August 5, 2024 15:37
Pietfried
Pietfried requested changes Aug 7, 2024
View reviewed changes
Copy link
Contributor

@Pietfried Pietfried left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👍

config/v16/config-docker.json
@@ -57,6 +57,48 @@
"ISO15118PnCEnabled": true,
"ContractValidationOffline": true
},
"CostAndPrice": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:
Our examples configs are pretty minimal so far, so I would suggest to set CustomDisplayCostAndPrice to false and remove all other keys.

I opened an issue to also provide a fully featured config as part of v16: #736

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok! i also changed config.json.

@maaikez
Review comments.
54cd5a1 
Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@maaikez
formatting...
262221a 
Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
@Pietfried Pietfried self-assigned this Aug 12, 2024
@maaikez maaikez merged commit 4a2f7a5 into main Aug 13, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Pietfried Pietfried Pietfried approved these changes

@hikinggrass hikinggrass Awaiting requested review from hikinggrass hikinggrass is a code owner

@marcemmers marcemmers Awaiting requested review from marcemmers marcemmers is a code owner

Assignees

@Pietfried Pietfried

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

OCPP 2.0.1 California Pricing Requirements: Tariff and Cost
2 participants
@maaikez @Pietfried