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

[shelly] Add support for Shelly BLU Gateway Gen3 #18174

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

markus7017
Copy link
Contributor

Adds a new device

@markus7017 markus7017 added the enhancement An enhancement or new feature for an existing add-on label Jan 24, 2025
@markus7017 markus7017 self-assigned this Jan 24, 2025
Signed-off-by: Markus Michels <[email protected]>
@markus7017 markus7017 requested review from lsiepel and removed request for lsiepel January 24, 2025 14:29
@markus7017 markus7017 added the work in progress A PR that is not yet ready to be merged label Jan 24, 2025
Copy link
Contributor

@lsiepel lsiepel left a comment

Choose a reason for hiding this comment

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

Ultra minor comment.
This PR not only adds support for new device it also changes de date time handling.

@@ -1607,6 +1609,7 @@ See notes on discovery of Shelly BLU devices above.
| battery | batteryLevel | Number | yes | Battery Level in % |
| | lowBattery | Switch | yes | Low battery alert (< 20%) |


Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct, I could if required create another PR

Copy link
Contributor

Choose a reason for hiding this comment

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

That would be best, thanks!

@markus7017 markus7017 removed the work in progress A PR that is not yet ready to be merged label Jan 24, 2025
@timbms
Copy link

timbms commented Jan 24, 2025

Will this also support BLU TRVs?

Copy link
Contributor

@jlaur jlaur left a comment

Choose a reason for hiding this comment

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

A few comments.

@@ -304,13 +304,17 @@ public static DateTimeType getTimestamp(String zone, long timestamp) {
ZoneId zoneId = !zone.isEmpty() ? ZoneId.of(zone) : ZoneId.systemDefault();
ZonedDateTime zdt = LocalDateTime.now().atZone(zoneId);
int delta = zdt.getOffset().getTotalSeconds();
return new DateTimeType(Instant.ofEpochSecond(timestamp - delta));
return new DateTimeType(ZonedDateTime.ofInstant(Instant.ofEpochSecond(timestamp - delta), zoneId));
Copy link
Contributor

Choose a reason for hiding this comment

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

This reverts a simplification from #17725, please revert:

Suggested change
return new DateTimeType(ZonedDateTime.ofInstant(Instant.ofEpochSecond(timestamp - delta), zoneId));
return new DateTimeType(Instant.ofEpochSecond(timestamp - delta));

See 03a0993.

@@ -345,7 +345,7 @@ private void addAttribute(Map<String, String> properties, ShellyManagerInterface
value = dateTimeState.format(null).replace('T', ' ').replace('-', '/');
break;
default:
value = dateTimeState.format(null).replace('T', ' ').replace('-', '/');
value = getTimestamp(dateTimeState);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this change here, but not in line 345? What is the desired format, do you actually want to use DateTimeType for formatting (openHAB), or ZonedDateTime (Java)?

In any case, not related to adding new device support, and looks like a half overwrite of 03a0993.

@jlaur jlaur changed the title [shelly] Support for Shelly BLU Gateway Gen3 [shelly] Add support for Shelly BLU Gateway Gen3 Jan 25, 2025
@jlaur jlaur added the awaiting feedback Awaiting feedback from the pull request author label Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting feedback Awaiting feedback from the pull request author enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants