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.

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

If occurred time is not initialised in BELGIUM_MAXIMUM_DEMAND_13_MONTHS default to timestamp of the month #140

Conversation

ejpalacios
Copy link
Contributor

For the entry BELGIUM_MAXIMUM_DEMAND_13_MONTHS , if there is no consumption at all during a given month, the value of the occurred field in that given month will have an invalid timestamp. Something like: 632525252525W.

See below the snipped of a telegram containing this type of reading. The change suggested in this PR is to default to the timestamp of the month.

0-0:98.1.0(9)(1-0:1.6.0)(1-0:1.6.0)(230101000000W)(632525252525W)(00.000*kW)(230201000000W)(632525252525W)(00.000*kW)(230301000000W)(632525252525W)(00.000*kW)(230401000000S)(230316134500W)(00.047*kW)(230601000000S)(230524154500S)(00.018*k
W)(230701000000S)(230626120000S)(00.008*kW)(230801000000S)(632525252525W)(00.000*kW)(230901000000S)(632525252525W)(00.000*kW)(231001000000S)(632525252525W)(00.000*kW)

It might not be something very common, but it can happen with meters that have been installed but not connected to end consumption point yet.

@dupondje
Copy link
Collaborator

dupondje commented Nov 2, 2023

Might want to prefer #143 ? As this will hide the value is not set by just setting a default value.
I might prefer to just handle the fact that the value can be None.

Copy link

@jbouwh jbouwh left a comment

Choose a reason for hiding this comment

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

@dupondje why would you want to fill in defaults. I think this simple fix works too.

@ejpalacios I am missing tests with this PR

@dupondje
Copy link
Collaborator

dupondje commented Nov 5, 2023

@dupondje why would you want to fill in defaults. I think this simple fix works too.

@jbouwh : this PR adds a default value if the occured timestamp can't be parsed. It might be cleaner to just handle the fact the value can be invalid and set it to None like #143 does.
Cause then the client of the library does know the value was non-existent/invalid instead of some default/current timestamp value.

@ejpalacios
Copy link
Contributor Author

@dupondje your approach makes more sense

@ejpalacios ejpalacios closed this Nov 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants