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

[SYS] Add timestamp for messages #1677

Merged
merged 2 commits into from Jun 15, 2023

Conversation

dkneisz
Copy link
Contributor

@dkneisz dkneisz commented Jun 11, 2023

Description:

  • add timestamp when publishing
  • add "GMT" as timezone for timestamp (e.g. for openHAB to use the correct time if you are in a different timezone)

Checklist:

  • The pull request is done against the latest development branch
  • Only one feature/fix was added per PR and the code change compiles without warnings
  • I accept the DCO.

- add timestamp when puglishing
- add "GMT" as timezone for timestamp
@1technophile
Copy link
Owner

1technophile commented Jun 14, 2023

Thanks for the PR. I think timestamp should be added in the joint pub function (from the main.ino) rather than adding it into each gateway.
This will be easier to maintain and will avoid code duplication.

@dkneisz
Copy link
Contributor Author

dkneisz commented Jun 14, 2023

I agree. Since it was already implemented in ZgatewayBT.ino I was under the impression, that it is implemented in some other modules also. That's why I just implemented it for ZgatewayRTL_433.ino. Should I implement this here or create another pull request?

@1technophile
Copy link
Owner

Up to you, I don't mind either solution

@dkneisz
Copy link
Contributor Author

dkneisz commented Jun 15, 2023

I moved it now to main.ino into the function "void pub(const char* topicori, JsonObject& data)". There are also to additional functions

  • void pub(const char* topicori, const char* payload, bool retainFlag)
  • void pub(const char* topicori, const char* payload)
    but they don't seem to do the same thing. I'm not quite sure where exactly they are used. They just publish the given payload to the given topic, so I didn't change anything there.

I tested this for the following:

  • BTtoMQTT
  • PilighttoMQTT
  • RLStoMQTT
  • SYStoMQTT
  • WebUItoMQTT
  • 433toMQTT
  • RTL_433toMQTT
  • SSD1306toMQTT

@1technophile
Copy link
Owner

Thanks

@1technophile 1technophile changed the title add timestamp for RTL_433 messages [SYS] Add timestamp for messages Jun 15, 2023
@1technophile 1technophile merged commit b5bc0f5 into 1technophile:development Jun 15, 2023
71 checks passed
@DigiH
Copy link
Collaborator

DigiH commented Jun 15, 2023

@dkneisz @1technophile

While great to have the time stamp option for all modules, why has the naming here changed from the definitive Z(ulu Time) to an ambiguous, non-existant, ZGMT naming scheme?

What we do get is Zulu time, which is also known as Coordinated Universal Time/Temps Universel Coordonné - abbreviated nonsensically as UTC, so either the previously existing Z or alternatively UTC should be used, but definitely not GMT in any form, as GMT does observe daylight saving time, which for the Coordinated Universal (Zulu) Time we give is incorrect and unwanted for users worldwide. Just to clarify my above comment.

@dkneisz
Copy link
Contributor Author

dkneisz commented Jun 16, 2023

@DigiH your are right. I was trying to fix an issue I had, that my openHAB used the wrong timezone for this date. Now that you mention it, I'm not sure why this solution worked for me. Even stranger I now reverted the change and my openHAB is now using my timezone. Not sure what happened here. I will revert this change and submit another pull request for this.

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.

None yet

3 participants