-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[chromecast] Refactoring of binding #7367
[chromecast] Refactoring of binding #7367
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't done a thorough review, though from what I saw most looks very good! 👍
...romecast/src/main/java/org/openhab/binding/chromecast/internal/utils/ByteArrayFileCache.java
Outdated
Show resolved
Hide resolved
@@ -17,6 +17,8 @@ thing-type.config.chromecast.device.ipAddress.label = IP-Adresse | |||
thing-type.config.chromecast.device.ipAddress.description = Lokale IP-Adresse oder Hostname des Chromecast Ger�tes. | |||
thing-type.config.chromecast.device.port.label = Port | |||
thing-type.config.chromecast.device.port.description = Port des Chromecast Ger�tes. | |||
thing-type.config.chromecast.device.refreshRate.label = Aktualisierungsintervall | |||
thing-type.config.chromecast.device.refreshRate.description = Intervall zur Aktualisierung des Chromecast Ger�tes. | |||
|
|||
# channel types | |||
channel-type.kodi.stop.label = Stop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should change this mistake too.
...ng.chromecast/src/main/java/org/openhab/binding/chromecast/internal/ChromecastAudioSink.java
Outdated
Show resolved
Hide resolved
...ng.chromecast/src/main/java/org/openhab/binding/chromecast/internal/ChromecastAudioSink.java
Show resolved
Hide resolved
...ng.chromecast/src/main/java/org/openhab/binding/chromecast/internal/ChromecastCommander.java
Show resolved
Hide resolved
...hromecast/src/main/java/org/openhab/binding/chromecast/internal/ChromecastStatusUpdater.java
Outdated
Show resolved
Hide resolved
...n/java/org/openhab/binding/chromecast/internal/discovery/ChromecastDiscoveryParticipant.java
Outdated
Show resolved
Hide resolved
...romecast/src/main/java/org/openhab/binding/chromecast/internal/utils/ByteArrayFileCache.java
Outdated
Show resolved
Hide resolved
final MessageDigest md = MessageDigest.getInstance(MD5_ALGORITHM); | ||
return String.format("%032x", new BigInteger(1, md.digest(key.getBytes(StandardCharsets.UTF_8)))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surely there are simpler ways to generate a unique file name than using an MD5 hash. Wouldn't the hashcode of the key suffice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you use this code in other bindings I'm assuming that it wouldn't be worth changing this...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing it here should be okay. But changing it in other bindings will result in "data loss" (data ist still there but not taken into account anymore). I am pretty sure there is no better way than using String.hashCode()
method at all.
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
cda5702
to
41c8294
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good enough
* Refactoring of binding * Incorporated comments from review Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de> Signed-off-by: Eugen Freiter <freiter@gmx.de>
* Refactoring of binding * Incorporated comments from review Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
* Refactoring of binding * Incorporated comments from review Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
* Refactoring of binding * Incorporated comments from review Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
* Refactoring of binding * Incorporated comments from review Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de> Signed-off-by: CSchlipp <christian@schlipp.de>
* Refactoring of binding * Incorporated comments from review Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
* Refactoring of binding * Incorporated comments from review Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
* Refactoring of binding * Incorporated comments from review Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
* Refactoring of binding * Incorporated comments from review Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
* Refactoring of binding * Incorporated comments from review Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de> Signed-off-by: Daan Meijer <daan@studioseptember.nl>
* Refactoring of binding * Incorporated comments from review Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Some stuff lying around in my branches.
Signed-off-by: Christoph Weitkamp github@christophweitkamp.de