datatime attribute added #58
Conversation
There was a problem hiding this comment.
Thanks a lot for your pull request!
I have only some minor findings.
Please also add a unit test to verify that reading the timestamp works.
If you want you could also change getDataTime() to return an Instant instead of long. This makes it easier to understand the meaning.
CHANGELOG.md
Outdated
|
|
||
| ## [1.6.1] - unreleased | ||
|
|
||
| - datatime attribute added in devicestats XML response since Fritz OS 7.50 |
There was a problem hiding this comment.
| - datatime attribute added in devicestats XML response since Fritz OS 7.50 | |
| - [#58](https://github.com/kaklakariada/fritzbox-java-api/pull/58) datatime attribute added in devicestats XML response since Fritz OS 7.50 |
There was a problem hiding this comment.
I've converted the return value into an Instant as you suggested and added a few small tests into your existing test classes.
Furthermore I've merged with your newest commit on master, hope you are fine with it.
In case I should add anything, let me know.
There was a problem hiding this comment.
Please avoid such a big diff and keep using * instead of - for lists
|
Hi @LutzHelling, I am not sure if the commit worked. I only see two additional merge commits in this PR, but there are no changes to the code. |
|
Hi,
I've commited to my fork only, I forgot to merge with upstream/master.
I hope now it's possible to see my changes.
Have a nice weekend
Lutz
…On Sat, Oct 7, 2023, 08:02 Christoph Pirkl ***@***.***> wrote:
Hi @LutzHelling <https://github.com/LutzHelling>, I am not sure if the
commit worked. I only see two additional merge commits in this PR, but
there are no changes to the code.
—
Reply to this email directly, view it on GitHub
<#58 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AN5LIATWP42SIXLKQCXYDXTX6DWBDAVCNFSM6AAAAAA5RBUT4CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONJRGYYTGNZTHE>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Hi @LutzHelling, |
|
@LutzHelling Please check my changes if this was your intention. |
Hi, I've added the datatime attribute when getting the energy consumption data. This helps plotting a consumption graph with time ticks on the x-axis. I would appreciate if you could integrate this extension in your beautiful project. Thanks in advance!