[mqtt.homeassistant] Fix MQTT Vacuum serialization names #11550
Conversation
| protected @Nullable String sendCommandTopic; // for custom_command | ||
| // variable names are with "_" by intention to avoid code cluttering with serialization annotations | ||
| protected @Nullable String command_topic; | ||
| protected String state_topic = ""; |
There was a problem hiding this comment.
You could use the @SerializedName("state_topic") annotation to point the parser to the right name. Then, you can stick to the code guidelines with the variable names.
There was a problem hiding this comment.
Did you see my comment? :)
// variable names are with "_" by intention to avoid code cluttering with serialization annotations
IMHO adding one annotation per variable is just spamming the code and makes it unreadable. If there would only be one or two such variables I would agree with you, but since there are more I would prefer to leave it like that.
Let me know what I should do, i.e. merge it or comment so I add some spam to the code... :)
There was a problem hiding this comment.
You can also call it future proofing instead of spam.
It is not uncommon that the default (de)serialization strategies change over time. 😉
There was a problem hiding this comment.
Fine. I have changed the MR to use annotations, please have a look again. Thanks.
There was a problem hiding this comment.
Thanks for changing it! The code might look less "spammy" if the DTO classes were in separate files. But that's a matter of personal taste.
There was a problem hiding this comment.
I agree, that was just copy&paste from other integrated homeassistant mqtt devices, hence all in one class+package ;)
While integrating the review comments, I did not remember that some variable names are written as they are because they result from some deserialization. This small PR fixes this. Signed-off-by: Stefan Triller <[email protected]>
While integrating the review comments, I did not remember that some variable names are written as they are because they result from some deserialization. This small PR fixes this. Signed-off-by: Stefan Triller <[email protected]>
While integrating the review comments, I did not remember that some variable names are written as they are because they result from some deserialization. This small PR fixes this. Signed-off-by: Stefan Triller <[email protected]> Signed-off-by: Nick Waterton <[email protected]>
While integrating the review comments, I did not remember that some variable names are written as they are because they result from some deserialization. This small PR fixes this. Signed-off-by: Stefan Triller <[email protected]> Signed-off-by: Michael Schmidt <[email protected]>
While integrating the review comments, I did not remember that some variable names are written as they are because they result from some deserialization. This small PR fixes this. Signed-off-by: Stefan Triller <[email protected]>
While integrating the review comments, I did not remember that some variable names are written as they are because they result from some deserialization. This small PR fixes this. Signed-off-by: Stefan Triller <[email protected]>
While integrating the review comments, I did not remember that some variable names are written as they are because they result from some deserialization. This small PR fixes this. Signed-off-by: Stefan Triller <[email protected]>
While integrating the review comments, I did not remember that some
variable names are written as they are because they result from some
deserialization.
This small PR fixes this.
Signed-off-by: Stefan Triller [email protected]