Add helpers to set charge point common data #1026
Conversation
The idea is to overwrite this data later, for example if other data sources are available than those available at the time of initialization. Signed-off-by: Michael Heimpold <[email protected]>
mhei
requested review from
Pietfried,
hikinggrass,
maaikez and
mlitre
as code owners
| void update_chargepoint_information(const std::string& chargepoint_vendor, const std::string& chargepoint_model, | ||
| const std::string& chargepoint_serialnumber, | ||
| const std::optional<std::string>& firmware_version); |
There was a problem hiding this comment.
nit: I would prefer vendor, model, serialnumber since `chargepoint is already part of the function name
| @@ -262,6 +262,18 @@ void ChargePointConfiguration::init_supported_measurands() { | |||
| } | |||
| } | |||
|
|
|||
| void ChargePointConfiguration::setChargepointInformation(const std::string& chargePointVendor, | |||
There was a problem hiding this comment.
Please add other properties of the BootNotification as well:
- iccid
- imsi
- meterSerialNumber
- meterType
- chargeBoxSerialNumber
There was a problem hiding this comment.
ok, will add as std::optional parameter
| this->config["Internal"]["ChargePointVendor"] = chargePointVendor; | ||
| this->config["Internal"]["ChargePointModel"] = chargePointModel; | ||
| this->config["Internal"]["ChargePointSerialNumber"] = chargePointSerialNumber; | ||
| if (firmwareVersion.has_value()) { | ||
| this->config["Internal"]["FirmwareVersion"] = firmwareVersion.value(); | ||
| } |
There was a problem hiding this comment.
In order to persist these changes they need to be written into the user_config.json. This can be done using the set_in_user_config function.
There was a problem hiding this comment.
I guess you refer to ChargePointConfiguration::setInUserConfig ? Hm, I'm unsure about this. When I call this function for each parameter, then this would result in multiple writes to the same file on disk. So if you insist that this should be made persistent, then I'd merge this functionality into a combined write.
But on the other hand: in my target use-case, it is not necessary to make these changes persistent since this function would be called during each boot and thus the data is always refreshed. Actually, the idea behind is that there is some common place outside the EVerest environment where such common charger data is stored permanently. Thus storing it again in the OCPP configuration file would not make sense, and especially not for e.g. IMSI or meter serial number (components which could be replaced during e.g maintenance...).
On the other hand, it would be written anyway as part of yet another key change...
There was a problem hiding this comment.
Yes, I was indeed refering to setInUserConfig.
I understand your use case, but libocpp itself is not aware of your target use case that calls the function on each boot, so I would like to treat the changes of these configuration keys like all the others. Otherwise this functionality should rather go into the start function or the ChargePoint constructor.
Of course you can merge the persisting functionality into a combined write. We are aware that the configuration handling for ocpp1.6 has some room for improvements in general.
Describe your changes
The idea is to overwrite this data later, for example if other data sources are available than those available at the time of initialization. This PR adds the required helper methods for the OCPP 1.6 implementation.
Note: since I currently only have a OCPP 1.6 test environment at hand and my knowledge about OCPP 2.x is still limited, help with OCPP 2.x part would be welcome.
Issue ticket number and link
Checklist before requesting a review