-
-
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
[panasonictv] Panasonic Viera TV Binding initial contribution #4407
Conversation
Signed-off-by: Prakashbabu Sidaraddi <e.prakashs@gmail.com> (github: scienty)
Would someone please do a code review and merge. Have been testing this continuously for 2 months in my home setup and it worked as expected. The code structure has been derived from samsungtv binding. |
You're not detecting online/offline of the TV as far as I can see, right? There is a subscription you can use and it will do a callback on power change to a URL (I think you can specify that URL aswell). My modification of the openhab1 binding simple has another webserver for that, it's not published anywhere though :D |
@Flole998 would you please provide sample code for that? I have been using it for couple of months with my Panasonic TV, even online and offline state for the thing is changed properly via UPNP service. I have a openhab rule configured to wait for the TV to come online from cold start, then check the switch status. Based on that the rule send keycode to switch appropriate HDMI. |
@cweitkamp @kaikreuzer @martinvw @wborn please review and approve the pull request. Appreciate your time. |
@cweitkamp @kaikreuzer @martinvw @wborn Need your precious time to get this into main stream. Please do the needful. Thanks, |
Sorry totally forgot about this, the code for having the TV push it's powerstate is this (it is in no way complete, perfect or even good, it's just a proof of concept or a hacky way I used for 2 years already). It requires some option to be enabled (I think) so the TV doesn't enter it's real power saving mode, my TV wakes up instantly when pushing the button and updates the status immediately:
|
Thanks @Flole998. Will check what improvements can be done with this and incorporate. |
Hello, https://github.com/openhab/openhab2-addons/issues/6104#issue-495903713 |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/panasonictv-binding-upgrade-to-openhab-2-x/82445/3 |
@scienty Did you have a chance to add the listener for power state changes? I think that could be one of the reasons this PR hasn't been reviewed yet. Unfortunately most of my code is "bad" or at least redundant (and the "integrated" OH Functions should be used instead (for example for getting the IP Address or serving a HTTP Request) based on "todays" OH2 standards, "back then" in OH1 days this was (almost) the only way to solve this and I never really got into OH2 development, otherwise I would have done this myself already. |
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.
@scienty thank you for your contributions. Apparently it took some time for someone to review. Sorry for that. I did an initial review. Most part of the code looks fine. Some methods are a bit complex, making it harder to understand what is going on.
This pr is already somewhat older and since then a number of things have changed. Most importantly the build system has changed. You need to migrate the binding first before getting it merged. There is a complete tutorial on how to migrate: https://community.openhab.org/t/tutorial-migrate-your-binding-to-the-maven-bnd-based-build-system/81389
*/ | ||
public class PanasonicTvHandler extends BaseThingHandler implements DiscoveryListener, RegistryListener, EventListener { | ||
|
||
private Logger logger = LoggerFactory.getLogger(PanasonicTvHandler.class); |
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.
private Logger logger = LoggerFactory.getLogger(PanasonicTvHandler.class); | |
private Logger final logger = LoggerFactory.getLogger(PanasonicTvHandler.class); |
public class PanasonicTvHandlerFactory extends BaseThingHandlerFactory { | ||
|
||
private static final Collection<ThingTypeUID> SUPPORTED_THING_TYPES_UIDS = Lists | ||
.newArrayList(PANASONIC_TV_THING_TYPE); |
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.
Remove dependency on google guava, use: Arrays.asList
.
String label = "Panasonic TV"; | ||
try { | ||
label = device.getDetails().getFriendlyName(); | ||
} catch (Exception e) { |
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.
Can you catch the specific exception here instead of the general Exception
? This looks like a lazy way to avoid null checks?
final ActionArgumentValue newArgument; | ||
try { | ||
newArgument = result.get(variable); | ||
} catch (final Exception ex) { |
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.
Can you catch the specific exception here?
if (newArgument.getValue() != null) { | ||
resultMap.put(variable, newArgument.getValue().toString()); | ||
} | ||
} catch (final Exception ex) { |
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.
Can you catch the specific exception here?
PanasonicTvUtils.buildHashMap("InstanceID", "0", "Channel", "Master")); | ||
updateResourceState("RenderingControl", "GetMute", | ||
PanasonicTvUtils.buildHashMap("InstanceID", "0", "Channel", "Master")); | ||
} catch (Exception e) { |
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.
Can you catch the specific exception?
*/ | ||
public class RemoteControllerService implements UpnpIOParticipant, PanasonicTvService { | ||
|
||
private Logger logger = LoggerFactory.getLogger(RemoteControllerService.class); |
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.
private Logger logger = LoggerFactory.getLogger(RemoteControllerService.class); | |
private final Logger logger = LoggerFactory.getLogger(RemoteControllerService.class); |
|
||
static final String SERVICE_NAME = "p00RemoteController"; | ||
|
||
private Map<String, String> stateMap = Collections.synchronizedMap(new HashMap<String, String>()); |
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.
private Map<String, String> stateMap = Collections.synchronizedMap(new HashMap<String, String>()); | |
private Map<String, String> stateMap = new ConcurrentHashMap<String, String>()); |
* | ||
* @param key Button code to send | ||
*/ | ||
private static final List<String> tvInputKeyCodes = Arrays.asList("NRC_HDMI1-ONOFF", "NRC_HDMI2-ONOFF", |
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.
Put all fields at the top of the class.
return null; | ||
} | ||
|
||
// TODO: remove manual connection |
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.
Remove this code if it's not used.
@scienty what is the status of this binding? Are you planning on finishing it? Thanks for your work! |
@scienty closing this pr due to long time no activity. If you still want to finish this work, which would be great, you can reopen this pr or if you need advice let us know. |
Expected Behavior
Enhancement, implement panasonictv binding for oh2 using upnp services
Legacy binding for panasonictv works in many cases but there are better features in oh2 to handle upnp communication and discovery
Fixes #3904