Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

[Core] Introduced discovery for USB devices with serial port #5315

Merged
merged 14 commits into from
Apr 5, 2018
Merged

[Core] Introduced discovery for USB devices with serial port #5315

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
2568d8c
[5153] Discovery for USB devices with serial port
hsudbrock Mar 1, 2018
5ce8b84
[5153] Address @amitjoy's review comment on synchronization of settin…
hsudbrock Mar 27, 2018
6530263
[5153] Addresses @amitjoy's review comment regarding setting the
hsudbrock Mar 27, 2018
663eeeb
[5153] Addresses @amitjoy's review comment regarding a more specific
hsudbrock Mar 27, 2018
8c50f72
[5153] Addresses @triller-telekom's review comments (except those with
hsudbrock Mar 28, 2018
2c3ff4c
[5153] Address review comments regarding the usbSerialDiscovery depen…
hsudbrock Mar 28, 2018
c5d1d9a
[5153] Address @triller-telekom's review comment regarding the use of
hsudbrock Mar 29, 2018
2c22d2f
[5153] Addresses @triller-telekom's comment regarding NonNullByDefault
hsudbrock Mar 29, 2018
6f6f739
[5153] Fix minor issues detected by the static code analysis
hsudbrock Mar 29, 2018
af5dd60
[5153] Fix failing test and make UsbSerialDiscoveryService config
hsudbrock Mar 29, 2018
177d8b9
[5153] Addresses @kaikreuzer's review comment to use linuxsysfs inste…
hsudbrock Mar 29, 2018
6115e2e
[5153] Addresses discussion with @triller-telekom regarding adding
hsudbrock Apr 4, 2018
9c422d3
[5153] Addresses the review comments by @sjka
hsudbrock Apr 4, 2018
e381290
[5153] Addresses the inclusion of USB interface information into the
hsudbrock Apr 5, 2018
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
46 changes: 32 additions & 14 deletions .../eclipse/smarthome/config/discovery/usbserial/internal/UsbSerialDiscoveryServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,9 @@ public void testSupportedThingTypesAreRetrievedFromDiscoveryParticipants() {

@Test
public void testThingsAreActuallyDiscovered() {
UsbSerialDiscoveryService service = getService(UsbSerialDiscoveryService.class);

// register one discovery listener
DiscoveryListener discoveryListener = mock(DiscoveryListener.class);
service.addDiscoveryListener(discoveryListener);
usbSerialDiscoveryService.addDiscoveryListener(discoveryListener);

// register two discovery participants
UsbSerialDiscoveryParticipant discoveryParticipantA = mock(UsbSerialDiscoveryParticipant.class);
Expand All @@ -116,47 +114,67 @@ public void testThingsAreActuallyDiscovered() {
// when no discovery participant supports a newly discovered device, no device is discovered
when(discoveryParticipantA.createResult(any())).thenReturn(null);
when(discoveryParticipantB.createResult(any())).thenReturn(null);
service.usbSerialDeviceDiscovered(generateDeviceInfo());
usbSerialDiscoveryService.usbSerialDeviceDiscovered(generateDeviceInfo());
verify(discoveryListener, never()).thingDiscovered(any(), any());

// when only the first discovery participant supports a newly discovered device, the device is discovered
UsbSerialDeviceInformation deviceInfoA = generateDeviceInfo();
DiscoveryResult discoveryResultA = mock(DiscoveryResult.class);
when(discoveryParticipantA.createResult(deviceInfoA)).thenReturn(discoveryResultA);
service.usbSerialDeviceDiscovered(deviceInfoA);
verify(discoveryListener, times(1)).thingDiscovered(service, discoveryResultA);
usbSerialDiscoveryService.usbSerialDeviceDiscovered(deviceInfoA);
verify(discoveryListener, times(1)).thingDiscovered(usbSerialDiscoveryService, discoveryResultA);

// when only the second discovery participant supports a newly discovered device, the device is also discovered
UsbSerialDeviceInformation deviceInfoB = generateDeviceInfo();
DiscoveryResult discoveryResultB = mock(DiscoveryResult.class);
when(discoveryParticipantA.createResult(deviceInfoB)).thenReturn(discoveryResultB);
service.usbSerialDeviceDiscovered(deviceInfoB);
verify(discoveryListener, times(1)).thingDiscovered(service, discoveryResultB);
usbSerialDiscoveryService.usbSerialDeviceDiscovered(deviceInfoB);
verify(discoveryListener, times(1)).thingDiscovered(usbSerialDiscoveryService, discoveryResultB);
}

@Test
public void testDiscoveredThingsAreRemoved() {
UsbSerialDiscoveryService service = getService(UsbSerialDiscoveryService.class);

// register one discovery listener
DiscoveryListener discoveryListener = mock(DiscoveryListener.class);
service.addDiscoveryListener(discoveryListener);
usbSerialDiscoveryService.addDiscoveryListener(discoveryListener);

// register one discovery participant
UsbSerialDiscoveryParticipant discoveryParticipant = mock(UsbSerialDiscoveryParticipant.class);
registerService(discoveryParticipant);

// when the discovery participant does not support a removed device, no discovery result is removed
when(discoveryParticipant.createResult(any())).thenReturn(null);
service.usbSerialDeviceRemoved(generateDeviceInfo());
usbSerialDiscoveryService.usbSerialDeviceRemoved(generateDeviceInfo());
verify(discoveryListener, never()).thingRemoved(any(), any());

// when the first discovery participant supports a removed device, the discovery result is removed
UsbSerialDeviceInformation deviceInfo = generateDeviceInfo();
ThingUID thingUID = mock(ThingUID.class);
when(discoveryParticipant.getThingUID(deviceInfo)).thenReturn(thingUID);
service.usbSerialDeviceRemoved(deviceInfo);
verify(discoveryListener, times(1)).thingRemoved(service, thingUID);
usbSerialDiscoveryService.usbSerialDeviceRemoved(deviceInfo);
verify(discoveryListener, times(1)).thingRemoved(usbSerialDiscoveryService, thingUID);
}

@Test
public void testAddingDiscoveryParticipantAfterAddingUsbDongle() {
UsbSerialDeviceInformation usb1 = generateDeviceInfo();
UsbSerialDeviceInformation usb2 = generateDeviceInfo();
UsbSerialDeviceInformation usb3 = generateDeviceInfo();

// get info about three added and one removed USB dongles from UsbSerialDiscovery
usbSerialDiscoveryService.usbSerialDeviceDiscovered(usb1);
usbSerialDiscoveryService.usbSerialDeviceDiscovered(usb2);
usbSerialDiscoveryService.usbSerialDeviceRemoved(usb1);
usbSerialDiscoveryService.usbSerialDeviceDiscovered(usb3);

// register one discovery participant
UsbSerialDiscoveryParticipant discoveryParticipant = mock(UsbSerialDiscoveryParticipant.class);
registerService(discoveryParticipant);

// then this discovery participant is informed about USB devices usb2 and usb3, but not about usb1
verify(discoveryParticipant, never()).createResult(usb1);
verify(discoveryParticipant, times(1)).createResult(usb2);
verify(discoveryParticipant, times(1)).createResult(usb3);
}

private void setBackgroundDiscovery(boolean status) throws IOException, InterruptedException {
Expand Down
11 changes: 11 additions & 0 deletions .../org/eclipse/smarthome/config/discovery/usbserial/internal/UsbSerialDiscoveryService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ public class UsbSerialDiscoveryService extends AbstractDiscoveryService implemen

private final Set<UsbSerialDiscoveryParticipant> discoveryParticipants = new CopyOnWriteArraySet<>();

private final Set<UsbSerialDeviceInformation> previouslyDiscovered = new CopyOnWriteArraySet<>();

@NonNullByDefault({})
private UsbSerialDiscovery usbSerialDiscovery;

Expand All @@ -86,6 +88,12 @@ protected void activate(@Nullable Map<String, @Nullable Object> configProperties
@Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC)
protected void addUsbSerialDiscoveryParticipant(UsbSerialDiscoveryParticipant participant) {
this.discoveryParticipants.add(participant);
for (UsbSerialDeviceInformation usbSerialDeviceInformation : previouslyDiscovered) {
DiscoveryResult result = participant.createResult(usbSerialDeviceInformation);
if (result != null) {
thingDiscovered(result);
}
}
}

protected void removeUsbSerialDiscoveryParticipant(UsbSerialDiscoveryParticipant participant) {
Expand All @@ -101,6 +109,7 @@ protected synchronized void unsetUsbSerialDiscovery(UsbSerialDiscovery usbSerial
usbSerialDiscovery.stopBackgroundScanning();
usbSerialDiscovery.unregisterDiscoveryListener(this);
this.usbSerialDiscovery = null;
this.previouslyDiscovered.clear();
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure that you want to clear the Set if you remove one participant? I think you have to leave the Set untouched if you remove a participant and only remove items from it in usbSerialDeviceRemoved, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@triller-telekom This method is called when unsetting the usbSerialDiscovery, not when removing a discoveryParticipant.

Copy link
Contributor

Choose a reason for hiding this comment

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

Github fooled me on this one... It folded away all lines between protected void removeUsbSerialDiscoveryParticipant(UsbSerialDiscoveryParticipant participant) { and usbSerialDiscovery.stopBackgroundScanning(); so I did not see that your added code ended up in unsetUsbSerialDiscovery. To me it looked as if you added it to removeUsbSerialDiscoveryParticipant.

So all is good now :)

}

@Modified
Expand Down Expand Up @@ -146,6 +155,7 @@ protected void stopBackgroundDiscovery() {
@Override
public void usbSerialDeviceDiscovered(UsbSerialDeviceInformation usbSerialDeviceInformation) {
logger.debug("Discovered new usb-serial device: {}", usbSerialDeviceInformation);
Copy link
Contributor

Choose a reason for hiding this comment

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

USB-serial

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will change.

previouslyDiscovered.add(usbSerialDeviceInformation);
for (UsbSerialDiscoveryParticipant participant : discoveryParticipants) {
DiscoveryResult result = participant.createResult(usbSerialDeviceInformation);
if (result != null) {
Expand All @@ -157,6 +167,7 @@ public void usbSerialDeviceDiscovered(UsbSerialDeviceInformation usbSerialDevice
@Override
public void usbSerialDeviceRemoved(UsbSerialDeviceInformation usbSerialDeviceInformation) {
logger.debug("Discovered removed usb-serial device: {}", usbSerialDeviceInformation);
previouslyDiscovered.remove(usbSerialDeviceInformation);
for (UsbSerialDiscoveryParticipant participant : discoveryParticipants) {
ThingUID thingUID = participant.getThingUID(usbSerialDeviceInformation);
if (thingUID != null) {
Expand Down