Allow omitting all optional SampledValue fields #140
Conversation
According to the OCPP specification, all fields of the SampledValue class except for "value" are optional and may be omitted. However, the Java class implementation in this library initialized all fields but "phase" to their default values, and prevented resetting all but the enum fields to null. Most prominently, the "unit" field was initialized to "Wh" and could not be reset, preventing the generation of compliant messages for "Frequency" and "Power.Factor" measurands, for which the "unit" field needs to be omitted. Remove the initialization of fields to their default values from the class constructors, so that they are left as null and will be omitted when serializing the object. Exempt null references from the validations in the field setter methods, so that all fields can be set to null. Add returning the default value in the field getter methods in case the stored field value is null, where applicable. Most importantly, for the "unit" field, only replace null with "Wh" for "Energy" type measurands. This ensures the null is retained for the "Frequency" and "Power.Factor" measurands, in compliance with the OCPP specification.
|
Hi @robert-s-ubi , Thanks for your contribution, I really appreciate it. For some reason the unit test for v2.0's boot notification is failing: I don't really understand why they fail, but I'll try to investigate it when I have more time (got some assignments due soon). |
|
Hi Thomas, I cannot reproduce any build or test failures, this commit builds with no issues on my machine: $ mvn test [...] Running eu.chargetime.ocpp.model.test.BootNotificationConfirmationTest [...] Running eu.chargetime.ocpp.model.basic.test.BootNotificationConfirmationTest [...] [INFO] --- maven-surefire-plugin:2.20.1:test (default-test) @ v2_0-test --- The Travis CI build is failing due to an issue with the coverall service_job_id, but that is only about uploading the coverage report, if I understand correctly. A "dry run" of the same command shown at the end of the Job log: mvn -DdryRun=true clean org.jacoco:jacoco-maven-plugin:prepare-agent test org.jacoco:jacoco-maven-plugin:report org.eluder.coveralls:coveralls-maven-plugin:report -B also finishes successfully. |
|
I'd really like to have this merged, as I have further changes and I would not want to have them pile up. Is there anything I can do to get this pull request further along? |
|
I'll merge it in now, so you can get on with your work. Thanks again for the contribution. |
According to the OCPP specification, all fields of the SampledValue
class except for "value" are optional and may be omitted.
However, the Java class implementation in this library initialized all
fields but "phase" to their default values, and prevented resetting all
but the enum fields to null. Most prominently, the "unit" field was
initialized to "Wh" and could not be reset, preventing the generation of
compliant messages for "Frequency" and "Power.Factor" measurands, for
which the "unit" field needs to be omitted.
Remove the initialization of fields to their default values from the
class constructors, so that they are left as null and will be omitted
when serializing the object.
Exempt null references from the validations in the field setter methods,
so that all fields can be set to null.
Add returning the default value in the field getter methods in case the
stored field value is null, where applicable.
Most importantly, for the "unit" field, only replace null with "Wh" for
"Energy" type measurands. This ensures the null is retained for the
"Frequency" and "Power.Factor" measurands, in compliance with the OCPP
specification.