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

[kodi] Thing handler factory with null annotations and constructor injection #8075

Merged
merged 2 commits into from
Jul 5, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.jetty.websocket.client.WebSocketClient;
import org.eclipse.smarthome.core.audio.AudioHTTPServer;
import org.eclipse.smarthome.core.audio.AudioSink;
Expand All @@ -33,6 +35,7 @@
import org.openhab.binding.kodi.internal.handler.KodiHandler;
import org.osgi.framework.ServiceRegistration;
import org.osgi.service.component.ComponentContext;
import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Reference;
import org.slf4j.Logger;
Expand All @@ -45,21 +48,32 @@
* @author Paul Frank - Initial contribution
* @author Christoph Weitkamp - Improvements on channels for opening PVR TV or Radio streams
*/
@NonNullByDefault
@Component(service = ThingHandlerFactory.class, configurationPid = "binding.kodi")
public class KodiHandlerFactory extends BaseThingHandlerFactory {

private Logger logger = LoggerFactory.getLogger(KodiHandlerFactory.class);
private final Logger logger = LoggerFactory.getLogger(KodiHandlerFactory.class);

private AudioHTTPServer audioHTTPServer;
private NetworkAddressService networkAddressService;
private WebSocketClient webSocketClient;
private final AudioHTTPServer audioHTTPServer;
private final NetworkAddressService networkAddressService;
private final KodiDynamicStateDescriptionProvider stateDescriptionProvider;
private final WebSocketClient webSocketClient;

// url (scheme+server+port) to use for playing notification sounds
private String callbackUrl = null;
private final Map<String, @Nullable ServiceRegistration<AudioSink>> audioSinkRegistrations = new ConcurrentHashMap<>();

private Map<String, ServiceRegistration<AudioSink>> audioSinkRegistrations = new ConcurrentHashMap<>();
// url (scheme+server+port) to use for playing notification sounds
private @Nullable String callbackUrl;

private KodiDynamicStateDescriptionProvider stateDescriptionProvider;
@Activate
public KodiHandlerFactory(final @Reference AudioHTTPServer audioHTTPServer,
final @Reference NetworkAddressService networkAddressService,
final @Reference KodiDynamicStateDescriptionProvider stateDescriptionProvider,
final @Reference WebSocketFactory webSocketFactory) {
this.audioHTTPServer = audioHTTPServer;
this.networkAddressService = networkAddressService;
this.stateDescriptionProvider = stateDescriptionProvider;
this.webSocketClient = webSocketFactory.getCommonWebSocketClient();
}

@Override
protected void activate(ComponentContext componentContext) {
Copy link
Contributor

@cweitkamp cweitkamp Jul 4, 2020

Choose a reason for hiding this comment

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

Please add @Activate annotation here too. Otherwise this method will be ignored in favor of all annotated activation methods.

Another option would be to pass the ComponentContext parameter to the constructor and move all the logic inside this method into the constructor too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Excuse me but I have a serious doubt. Please look at the handler factory for the Sonos and LGwebOS bindings for example. They are done like that. If activate method was not called, the LGwebOS binding would just not work at all a(s the WebSocket client would not be started).

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, interesting. I do not have a clue why it is working but it seems to work.

Expand All @@ -74,7 +88,7 @@ public boolean supportsThingType(ThingTypeUID thingTypeUID) {
}

@Override
protected ThingHandler createHandler(Thing thing) {
protected @Nullable ThingHandler createHandler(Thing thing) {
ThingTypeUID thingTypeUID = thing.getThingTypeUID();

if (thingTypeUID.equals(THING_TYPE_KODI)) {
Expand All @@ -94,7 +108,7 @@ protected ThingHandler createHandler(Thing thing) {
return null;
}

private String createCallbackUrl() {
private @Nullable String createCallbackUrl() {
if (callbackUrl != null) {
return callbackUrl;
} else {
Expand Down Expand Up @@ -123,40 +137,4 @@ public void unregisterHandler(Thing thing) {
reg.unregister();
}
}

@Reference
protected void setHttpClientFactory(WebSocketFactory webSocketFactory) {
this.webSocketClient = webSocketFactory.getCommonWebSocketClient();
}

protected void unsetHttpClientFactory(WebSocketFactory webSocketFactory) {
this.webSocketClient = null;
}

@Reference
protected void setAudioHTTPServer(AudioHTTPServer audioHTTPServer) {
this.audioHTTPServer = audioHTTPServer;
}

protected void unsetAudioHTTPServer(AudioHTTPServer audioHTTPServer) {
this.audioHTTPServer = null;
}

@Reference
protected void setNetworkAddressService(NetworkAddressService networkAddressService) {
this.networkAddressService = networkAddressService;
}

protected void unsetNetworkAddressService(NetworkAddressService networkAddressService) {
this.networkAddressService = null;
}

@Reference
protected void setDynamicStateDescriptionProvider(KodiDynamicStateDescriptionProvider stateDescriptionProvider) {
this.stateDescriptionProvider = stateDescriptionProvider;
}

protected void unsetDynamicStateDescriptionProvider(KodiDynamicStateDescriptionProvider stateDescriptionProvider) {
this.stateDescriptionProvider = null;
}
}