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

[amazonechocontrol] Broken last voice command on Fire TV Cube #592

Merged
merged 7 commits into from
Oct 13, 2024

Conversation

Trinitus01
Copy link
Contributor

fixed: boken last voice command on Fire TV Cube

Signed-off-by: Tom Blum [email protected]

@Trinitus01 Trinitus01 requested a review from J-N-K as a code owner October 4, 2024 15:25
Signed-off-by: Tom Blum <[email protected]>
Copy link
Member

@J-N-K J-N-K left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM

@@ -141,6 +144,8 @@ public class AccountHandler extends BaseBridgeHandler implements PushConnection.
private boolean disposing = false;
private @Nullable AccountTO accountInformation;

Set<String> deviceSerialNumbers = Collections.synchronizedSet(new LinkedHashSet<>());
Copy link
Member

Choose a reason for hiding this comment

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

Why don't you use ConcurrentHashMap.newKeySet()? That's what we usually do if we want to create a new thread-safe Set.

Copy link
Member

Choose a reason for hiding this comment

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

@Trinitus01 The "final" solution CopyOnWriteArraySet does not work, because it's iterator does not support remove and throws an UnsupportedOperationException. Is there any reason why this is a class field and not a local variable? As far as I can see that would also be possible.

Copy link
Contributor Author

@Trinitus01 Trinitus01 Jan 19, 2025

Choose a reason for hiding this comment

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

It seems there is no reason why this is a class field. So we can change it to a local one.

Signed-off-by: Jan N. Klug <[email protected]>
@J-N-K J-N-K added the bug Something isn't working label Oct 13, 2024
@J-N-K J-N-K merged commit 5e77661 into smarthomej:4.1.x Oct 13, 2024
1 of 2 checks passed
J-N-K added a commit that referenced this pull request Oct 13, 2024
* fixed: missing Content-Length header (BodyPublisher.ofByteArrays JDK 17 issue)

Signed-off-by: Tom Blum [email protected]

* fixed: commited line endings

Signed-off-by: Tom Blum [email protected]

* removed: logger

Signed-off-by: Tom Blum [email protected]

* fixed: boken last voice command on Fire TV Cube

Signed-off-by: Tom Blum <[email protected]>

* fixed: spotless aso

Signed-off-by: Tom Blum <[email protected]>

* review

Signed-off-by: Jan N. Klug <[email protected]>

---------

Signed-off-by: Tom Blum [email protected]
Signed-off-by: Tom Blum <[email protected]>
Signed-off-by: Jan N. Klug <[email protected]>
Co-authored-by: Jan N. Klug <[email protected]>
Signed-off-by: Jan N. Klug <[email protected]>
J-N-K added a commit that referenced this pull request Oct 13, 2024
* fixed: missing Content-Length header (BodyPublisher.ofByteArrays JDK 17 issue)

Signed-off-by: Tom Blum [email protected]

* fixed: commited line endings

Signed-off-by: Tom Blum [email protected]

* removed: logger

Signed-off-by: Tom Blum [email protected]

* fixed: boken last voice command on Fire TV Cube

Signed-off-by: Tom Blum <[email protected]>

* fixed: spotless aso

Signed-off-by: Tom Blum <[email protected]>

* review

Signed-off-by: Jan N. Klug <[email protected]>

---------

Signed-off-by: Tom Blum [email protected]
Signed-off-by: Tom Blum <[email protected]>
Signed-off-by: Jan N. Klug <[email protected]>
Co-authored-by: Jan N. Klug <[email protected]>
Signed-off-by: Jan N. Klug <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants