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

[emotiva] Fix missing data in source channels #17365

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
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 @@ -14,6 +14,7 @@

import static org.openhab.binding.emotiva.internal.EmotivaBindingConstants.*;

import java.util.EnumMap;
import java.util.Map;

import org.eclipse.jdt.annotation.NonNullByDefault;
Expand Down Expand Up @@ -70,7 +71,7 @@ private static int clamp(int volumeInPercentage, int min, int max) {
}

public static EmotivaControlRequest channelToControlRequest(String id,
Map<String, Map<EmotivaControlCommands, String>> commandMaps, EmotivaProtocolVersion protocolVersion) {
Map<String, EnumMap<EmotivaControlCommands, String>> commandMaps, EmotivaProtocolVersion protocolVersion) {
EmotivaSubscriptionTags channelSubscription = EmotivaSubscriptionTags.fromChannelUID(id);
EmotivaControlCommands channelFromCommand = OHChannelToEmotivaCommand.fromChannelUID(id);
return new EmotivaControlRequest(id, channelSubscription, channelFromCommand, commandMaps, protocolVersion);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ScheduledFuture;
Expand Down Expand Up @@ -108,15 +109,16 @@ public class EmotivaProcessorHandler extends BaseThingHandler {

/**
* Emotiva devices have trouble with too many subscriptions in same request, so subscriptions are dividing into
* those general group channels, and the rest.
* groups.
*/
private final EmotivaSubscriptionTags[] generalSubscription = EmotivaSubscriptionTags.generalChannels();
private final EmotivaSubscriptionTags[] nonGeneralSubscriptions = EmotivaSubscriptionTags.nonGeneralChannels();
private final List<EmotivaSubscriptionTags> generalSubscription = EmotivaSubscriptionTags.channels("general");
private final List<EmotivaSubscriptionTags> mainZoneSubscriptions = EmotivaSubscriptionTags.channels("main-zone");
private final List<EmotivaSubscriptionTags> zone2Subscriptions = EmotivaSubscriptionTags.channels("zone2");

private final EnumMap<EmotivaControlCommands, String> sourcesMainZone;
private final EnumMap<EmotivaControlCommands, String> sourcesZone2;
private final EnumMap<EmotivaSubscriptionTags, String> modes;
private final Map<String, Map<EmotivaControlCommands, String>> commandMaps = new ConcurrentHashMap<>();
private final Map<String, EnumMap<EmotivaControlCommands, String>> commandMaps = new ConcurrentHashMap<>();
private final EmotivaTranslationProvider i18nProvider;

private @Nullable ScheduledFuture<?> pollingJob;
Expand Down Expand Up @@ -222,7 +224,8 @@ private synchronized void connect() {
try {
logger.debug("Connection attempt '{}'", attempt);
sendConnector.sendSubscription(generalSubscription, config);
sendConnector.sendSubscription(nonGeneralSubscriptions, config);
sendConnector.sendSubscription(mainZoneSubscriptions, config);
sendConnector.sendSubscription(zone2Subscriptions, config);
} catch (IOException e) {
// network or socket failure, also wait 2 sec and try again
}
Expand Down Expand Up @@ -529,11 +532,10 @@ private void handleChannelUpdate(String emotivaSubscriptionName, String value, S
// Add/Update user assigned name for inputs
if (subscriptionTag.getChannel().startsWith(CHANNEL_INPUT1.substring(0, CHANNEL_INPUT1.indexOf("-") + 1))
&& "true".equals(visible)) {
logger.debug("Adding '{}' to dynamic source input list", trimmedValue);
sourcesMainZone.put(EmotivaControlCommands.matchToInput(subscriptionTag.name()), trimmedValue);
commandMaps.put(MAP_SOURCES_MAIN_ZONE, sourcesMainZone);

logger.debug("sources list is now {}", sourcesMainZone.size());
logger.debug("Adding '{}' to dynamic source input list, map is now {}", trimmedValue,
commandMaps.get(MAP_SOURCES_MAIN_ZONE));
}

// Add/Update audio modes
Expand Down Expand Up @@ -715,7 +717,8 @@ private synchronized void disconnect() {
try {
// Unsubscribe before disconnect
localSendingService.sendUnsubscribe(generalSubscription);
localSendingService.sendUnsubscribe(nonGeneralSubscriptions);
localSendingService.sendUnsubscribe(mainZoneSubscriptions);
localSendingService.sendUnsubscribe(zone2Subscriptions);
} catch (IOException e) {
logger.debug("Failed to unsubscribe for '{}'", config.ipAddress, e);
}
Expand Down Expand Up @@ -773,11 +776,14 @@ public Collection<Class<? extends ThingHandlerService>> getServices() {
}

public EnumMap<EmotivaControlCommands, String> getSourcesMainZone() {
return sourcesMainZone;
EnumMap<EmotivaControlCommands, String> localMainZoneSourcesMap = commandMaps.get(MAP_SOURCES_MAIN_ZONE);
return localMainZoneSourcesMap == null || localMainZoneSourcesMap.isEmpty() ? sourcesMainZone
: localMainZoneSourcesMap;
}

public EnumMap<EmotivaControlCommands, String> getSourcesZone2() {
return sourcesZone2;
EnumMap<EmotivaControlCommands, String> localZone2SourcesMap = commandMaps.get(MAP_SOURCES_ZONE_2);
return Objects.requireNonNullElse(localZone2SourcesMap, sourcesZone2);
}

public EnumMap<EmotivaSubscriptionTags, String> getModes() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,17 +156,17 @@ private void listenUnhandledInterruption() throws InterruptedException {
localReceivingSocket.receive(answer); // receive packet (blocking call)
listenerNotifyActive = false;

final byte[] receivedData = Arrays.copyOfRange(answer.getData(), 0, answer.getLength() - 1);
final int receivedDataLength = Arrays.copyOfRange(answer.getData(), 0, answer.getLength() - 1).length;

if (receivedData.length == 0) {
if (receivedDataLength == 0) {
if (isConnected()) {
logger.debug("Nothing received, this may happen during shutdown or some unknown error");
}
continue;
}
receiveNotifyFailures = 0; // message successfully received, unset failure counter

handleReceivedData(answer, receivedData, localListener);
handleReceivedData(answer, localListener);
} catch (Exception e) {
listenerNotifyActive = false;

Expand All @@ -190,12 +190,11 @@ private void listenUnhandledInterruption() throws InterruptedException {
}
}

private void handleReceivedData(DatagramPacket answer, byte[] receivedData,
Consumer<EmotivaUdpResponse> localListener) {
private void handleReceivedData(DatagramPacket answer, Consumer<EmotivaUdpResponse> localListener) {
// log & notify listener in new thread (so that listener loop continues immediately)
executorService.execute(() -> {
if (answer.getAddress() != null && answer.getLength() > 0) {
logger.trace("Received data on port '{}': {}", answer.getPort(), receivedData);
logger.debug("Received data on port '{}'", answer.getPort());
EmotivaUdpResponse emotivaUdpResponse = new EmotivaUdpResponse(
new String(answer.getData(), 0, answer.getLength()), answer.getAddress().getHostAddress());
localListener.accept(emotivaUdpResponse);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.net.SocketException;
import java.nio.charset.Charset;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.concurrent.ExecutorService;
import java.util.function.Consumer;
Expand Down Expand Up @@ -122,12 +123,11 @@ public void connect(Consumer<EmotivaUdpResponse> listener, boolean logNotThrowEx
}
}

private void handleReceivedData(DatagramPacket answer, byte[] receivedData,
Consumer<EmotivaUdpResponse> localListener) {
private void handleReceivedData(DatagramPacket answer, Consumer<EmotivaUdpResponse> localListener) {
// log & notify listener in new thread (so that listener loop continues immediately)
executorService.execute(() -> {
if (answer.getAddress() != null && answer.getLength() > 0) {
logger.trace("Received data on port '{}': {}", answer.getPort(), receivedData);
logger.trace("Received data on port '{}'", answer.getPort());
espenaf marked this conversation as resolved.
Show resolved Hide resolved
EmotivaUdpResponse emotivaUdpResponse = new EmotivaUdpResponse(
new String(answer.getData(), 0, answer.getLength()), answer.getAddress().getHostAddress());
localListener.accept(emotivaUdpResponse);
Expand Down Expand Up @@ -158,7 +158,7 @@ public void send(EmotivaControlDTO dto) throws IOException {
send(emotivaXmlUtils.marshallJAXBElementObjects(dto));
}

public void sendSubscription(EmotivaSubscriptionTags[] tags, EmotivaConfiguration config) throws IOException {
public void sendSubscription(List<EmotivaSubscriptionTags> tags, EmotivaConfiguration config) throws IOException {
send(emotivaXmlUtils.marshallJAXBElementObjects(new EmotivaSubscriptionRequest(tags, config.protocolVersion)));
}

Expand All @@ -171,7 +171,7 @@ public void sendUpdate(EmotivaSubscriptionTags[] tags, EmotivaConfiguration conf
send(emotivaXmlUtils.marshallJAXBElementObjects(new EmotivaUpdateRequest(tags, config.protocolVersion)));
}

public void sendUnsubscribe(EmotivaSubscriptionTags[] defaultCommand) throws IOException {
public void sendUnsubscribe(List<EmotivaSubscriptionTags> defaultCommand) throws IOException {
send(emotivaXmlUtils.marshallJAXBElementObjects(new EmotivaUnsubscribeDTO(defaultCommand)));
}

Expand All @@ -196,14 +196,14 @@ public void send(String msg) throws IOException {
logger.debug("Sending successful");

localDatagramSocket.receive(answer);
final byte[] receivedData = Arrays.copyOfRange(answer.getData(), 0, answer.getLength() - 1);
final int receivedDataLength = Arrays.copyOfRange(answer.getData(), 0, answer.getLength() - 1).length;

if (receivedData.length == 0) {
if (receivedDataLength == 0) {
logger.debug("Nothing received, this may happen during shutdown or some unknown error");
}

if (localListener != null) {
handleReceivedData(answer, receivedData, localListener);
handleReceivedData(answer, localListener);
}
} else {
throw new SocketException("Datagram Socket closed or not initialized");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public void setThingHandler(ThingHandler handler) {
public @Nullable StateDescription getStateDescription(Channel channel, @Nullable StateDescription original,
@Nullable Locale locale) {
ChannelTypeUID typeUID = channel.getChannelTypeUID();
if (typeUID == null || !BINDING_ID.equals(typeUID.getBindingId()) || original == null) {
if (typeUID == null || !BINDING_ID.equals(typeUID.getBindingId())) {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,12 @@

import static org.openhab.binding.emotiva.internal.protocol.EmotivaProtocolVersion.PROTOCOL_V2;

import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;

import javax.xml.bind.annotation.XmlAttribute;
import javax.xml.bind.annotation.XmlRootElement;

import org.openhab.binding.emotiva.internal.protocol.EmotivaControlCommands;
import org.openhab.binding.emotiva.internal.protocol.EmotivaSubscriptionTags;

/**
Expand All @@ -38,26 +37,18 @@ public class EmotivaSubscriptionRequest extends AbstractJAXBElementDTO {
public EmotivaSubscriptionRequest() {
}

public EmotivaSubscriptionRequest(List<EmotivaCommandDTO> commands, String protocol) {
public EmotivaSubscriptionRequest(List<EmotivaSubscriptionTags> emotivaCommandTypes, String protocol) {
this.protocol = protocol;
this.commands = commands;
}

public EmotivaSubscriptionRequest(EmotivaSubscriptionTags[] emotivaCommandTypes, String protocol) {
this.protocol = protocol;
List<EmotivaCommandDTO> list = new ArrayList<>();
for (EmotivaSubscriptionTags commandType : emotivaCommandTypes) {
list.add(EmotivaCommandDTO.fromTypeWithAck(commandType));
}
this.commands = list;
this.commands = emotivaCommandTypes.stream().map(EmotivaCommandDTO::fromTypeWithAck)
.collect(Collectors.toList());
}

public EmotivaSubscriptionRequest(EmotivaSubscriptionTags tag) {
this.commands = List.of(EmotivaCommandDTO.fromTypeWithAck(tag));
}

public EmotivaSubscriptionRequest(EmotivaControlCommands commandType, String protocol) {
public EmotivaSubscriptionRequest(EmotivaCommandDTO commandType, String protocol) {
this.protocol = protocol;
this.commands = List.of(EmotivaCommandDTO.fromTypeWithAck(commandType));
this.commands = List.of(commandType);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@

import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;

import javax.xml.bind.annotation.XmlRootElement;

import org.openhab.binding.emotiva.internal.protocol.EmotivaControlCommands;
import org.openhab.binding.emotiva.internal.protocol.EmotivaSubscriptionTags;

/**
Expand All @@ -34,10 +34,6 @@ public class EmotivaUnsubscribeDTO extends AbstractJAXBElementDTO {
public EmotivaUnsubscribeDTO() {
}

public EmotivaUnsubscribeDTO(List<EmotivaCommandDTO> commands) {
this.commands = commands;
}

public EmotivaUnsubscribeDTO(EmotivaSubscriptionTags[] emotivaCommandTypes) {
List<EmotivaCommandDTO> list = new ArrayList<>();
for (EmotivaSubscriptionTags commandType : emotivaCommandTypes) {
Expand All @@ -50,7 +46,11 @@ public EmotivaUnsubscribeDTO(EmotivaSubscriptionTags tag) {
this.commands = List.of(EmotivaCommandDTO.fromType(tag));
}

public EmotivaUnsubscribeDTO(EmotivaControlCommands commandType) {
this.commands = List.of(EmotivaCommandDTO.fromType(commandType));
public EmotivaUnsubscribeDTO(EmotivaCommandDTO commandType) {
this.commands = List.of(commandType);
}

public EmotivaUnsubscribeDTO(List<EmotivaSubscriptionTags> commandType) {
this.commands = commandType.stream().map(EmotivaCommandDTO::fromTypeWithAck).collect(Collectors.toList());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static org.openhab.binding.emotiva.internal.protocol.EmotivaSubscriptionTags.tuner_band;
import static org.openhab.binding.emotiva.internal.protocol.EmotivaSubscriptionTags.tuner_channel;

import java.util.EnumMap;
import java.util.Map;

import org.eclipse.jdt.annotation.NonNullByDefault;
Expand Down Expand Up @@ -53,11 +54,11 @@ public class EmotivaControlRequest {
private final EmotivaControlCommands downCommand;
private double maxValue;
private double minValue;
private final Map<String, Map<EmotivaControlCommands, String>> commandMaps;
private final Map<String, EnumMap<EmotivaControlCommands, String>> commandMaps;
private final EmotivaProtocolVersion protocolVersion;

public EmotivaControlRequest(String channel, EmotivaSubscriptionTags channelSubscription,
EmotivaControlCommands controlCommand, Map<String, Map<EmotivaControlCommands, String>> commandMaps,
EmotivaControlCommands controlCommand, Map<String, EnumMap<EmotivaControlCommands, String>> commandMaps,
EmotivaProtocolVersion protocolVersion) {
if (channelSubscription.equals(EmotivaSubscriptionTags.unknown)) {
if (controlCommand.equals(EmotivaControlCommands.none)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,24 +132,14 @@ public static EmotivaSubscriptionTags fromChannelUID(String id) {
return EmotivaSubscriptionTags.unknown;
}

public static EmotivaSubscriptionTags[] generalChannels() {
public static List<EmotivaSubscriptionTags> channels(String zonePrefix) {
List<EmotivaSubscriptionTags> tags = new ArrayList<>();
for (EmotivaSubscriptionTags value : values()) {
if (value.channel.startsWith("general")) {
if (value.channel.startsWith(zonePrefix)) {
tags.add(value);
}
}
return tags.toArray(new EmotivaSubscriptionTags[0]);
}

public static EmotivaSubscriptionTags[] nonGeneralChannels() {
List<EmotivaSubscriptionTags> tags = new ArrayList<>();
for (EmotivaSubscriptionTags value : values()) {
if (!value.channel.startsWith("general")) {
tags.add(value);
}
}
return tags.toArray(new EmotivaSubscriptionTags[0]);
return tags;
}

public static EmotivaSubscriptionTags[] speakerChannels() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import static org.openhab.binding.emotiva.internal.protocol.EmotivaProtocolVersion.PROTOCOL_V2;
import static org.openhab.binding.emotiva.internal.protocol.EmotivaProtocolVersion.PROTOCOL_V3;

import java.util.EnumMap;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Stream;
Expand Down Expand Up @@ -90,7 +91,7 @@ private static Stream<Arguments> channelToControlRequest() {
void testChannelToControlRequest(String channel, String name, EmotivaDataType emotivaDataType,
EmotivaControlCommands defaultCommand, EmotivaControlCommands onCommand, EmotivaControlCommands offCommand,
EmotivaControlCommands setCommand, EmotivaProtocolVersion version, double min, double max) {
final Map<String, Map<EmotivaControlCommands, String>> commandMaps = new ConcurrentHashMap<>();
final Map<String, EnumMap<EmotivaControlCommands, String>> commandMaps = new ConcurrentHashMap<>();

EmotivaControlRequest surround = EmotivaCommandHelper.channelToControlRequest(channel, commandMaps, version);
assertThat(surround.getName(), is(name));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,16 @@ void marshalFromChannelUID() {
}

@Test
void marshallWithTwoSubscriptionsNoAck() {
EmotivaCommandDTO command1 = new EmotivaCommandDTO(EmotivaControlCommands.volume, "10", "yes");
EmotivaCommandDTO command2 = new EmotivaCommandDTO(EmotivaControlCommands.power_off);
void marshallWithSubscriptionNoAck() {
EmotivaCommandDTO command = new EmotivaCommandDTO(EmotivaControlCommands.volume, "10", "yes");

EmotivaSubscriptionRequest dto = new EmotivaSubscriptionRequest(List.of(command1, command2),
PROTOCOL_V2.value());
EmotivaSubscriptionRequest dto = new EmotivaSubscriptionRequest(command, PROTOCOL_V2.value());

String xmlString = xmlUtils.marshallJAXBElementObjects(dto);
assertThat(xmlString, containsString("<emotivaSubscription protocol=\"2.0\">"));
assertThat(xmlString, containsString("<volume value=\"10\" ack=\"yes\" />"));
assertThat(xmlString, containsString("<power_off />"));
assertThat(xmlString, containsString("</emotivaSubscription>"));
assertThat(xmlString, not(containsString("<volume>")));
assertThat(xmlString, not(containsString("<command>")));
}

@Test
Expand Down
Loading