Skip to content

Commit

Permalink
[boschshc] Code enhancements and Sonar issue fixes (#15160)
Browse files Browse the repository at this point in the history
* replace call to deprecated HSBType::getRGB() with
ColorUtil::hsbTosRgb()
* make constructors of abstract classes protected
* use instanceof with pattern matching (JEP 305)
* enhance switches with combined cases using comma-separated case
expressions
* remove unnecessary public modifiers in unit tests

Signed-off-by: David Pace <dev@davidpace.de>
  • Loading branch information
david-pace authored Jul 4, 2023
1 parent 4775ad2 commit 123982d
Show file tree
Hide file tree
Showing 34 changed files with 140 additions and 144 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public abstract class AbstractBatteryPoweredDeviceHandler extends BoschSHCDevice
*/
private final BatteryLevelService batteryLevelService;

public AbstractBatteryPoweredDeviceHandler(Thing thing) {
protected AbstractBatteryPoweredDeviceHandler(Thing thing) {
super(thing);
this.batteryLevelService = new BatteryLevelService();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ public void handleCommand(ChannelUID channelUID, Command command) {

switch (channelUID.getId()) {
case CHANNEL_POWER_SWITCH:
if (command instanceof OnOffType) {
updatePowerSwitchState((OnOffType) command);
if (command instanceof OnOffType onOffCommand) {
updatePowerSwitchState(onOffCommand);
}
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public abstract class AbstractSmokeDetectorHandler extends AbstractBatteryPowere

private SmokeDetectorCheckService smokeDetectorCheckService;

public AbstractSmokeDetectorHandler(Thing thing) {
protected AbstractSmokeDetectorHandler(Thing thing) {
super(thing);
this.smokeDetectorCheckService = new SmokeDetectorCheckService();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@
*/
package org.openhab.binding.boschshc.internal.devices.camera;

import static org.openhab.binding.boschshc.internal.devices.BoschSHCBindingConstants.CHANNEL_CAMERA_NOTIFICATION;
import static org.openhab.binding.boschshc.internal.devices.BoschSHCBindingConstants.CHANNEL_PRIVACY_MODE;
import static org.openhab.binding.boschshc.internal.devices.BoschSHCBindingConstants.*;

import java.util.List;

Expand All @@ -35,18 +34,18 @@
* Handler for security cameras.
* <p>
* This implementation handles services and commands that are common to all cameras, which are currently:
*
*
* <ul>
* <li><code>PrivacyMode</code> - Controls whether the camera records images</li>
* <li><code>CameraNotification</code> - Enables or disables notifications for the camera</li>
* </ul>
*
*
* <p>
* The Eyes outdoor camera advertises a <code>CameraLight</code> service, which unfortunately does not work properly.
* Valid states are <code>ON</code> and <code>OFF</code>.
* One of my two cameras returns <code>HTTP 204 (No Content)</code> when requesting the state.
* Once Bosch supports this service properly, a new subclass may be introduced for the Eyes outdoor camera.
*
*
* @author David Pace - Initial contribution
*
*/
Expand Down Expand Up @@ -77,14 +76,14 @@ public void handleCommand(ChannelUID channelUID, Command command) {

switch (channelUID.getId()) {
case CHANNEL_PRIVACY_MODE:
if (command instanceof OnOffType) {
updatePrivacyModeState((OnOffType) command);
if (command instanceof OnOffType onOffCommand) {
updatePrivacyModeState(onOffCommand);
}
break;

case CHANNEL_CAMERA_NOTIFICATION:
if (command instanceof OnOffType) {
updateCameraNotificationState((OnOffType) command);
if (command instanceof OnOffType onOffCommand) {
updateCameraNotificationState(onOffCommand);
}
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,31 +71,28 @@ protected void initializeServices() throws BoschSHCException {
public void handleCommand(ChannelUID channelUID, Command command) {
super.handleCommand(channelUID, command);

if (command instanceof UpDownType) {
if (command instanceof UpDownType upDownCommand) {
// Set full close/open as target state
UpDownType upDownType = (UpDownType) command;
ShutterControlServiceState state = new ShutterControlServiceState();
if (upDownType == UpDownType.UP) {
if (upDownCommand == UpDownType.UP) {
state.level = 1.0;
} else if (upDownType == UpDownType.DOWN) {
} else if (upDownCommand == UpDownType.DOWN) {
state.level = 0.0;
} else {
logger.warn("Received unknown UpDownType command: {}", upDownType);
logger.warn("Received unknown UpDownType command: {}", upDownCommand);
return;
}
this.updateServiceState(this.shutterControlService, state);
} else if (command instanceof StopMoveType) {
StopMoveType stopMoveType = (StopMoveType) command;
if (stopMoveType == StopMoveType.STOP) {
} else if (command instanceof StopMoveType stopMoveCommand) {
if (stopMoveCommand == StopMoveType.STOP) {
// Set STOPPED operation state
ShutterControlServiceState state = new ShutterControlServiceState();
state.operationState = OperationState.STOPPED;
this.updateServiceState(this.shutterControlService, state);
}
} else if (command instanceof PercentType) {
} else if (command instanceof PercentType percentCommand) {
// Set specific level
PercentType percentType = (PercentType) command;
double level = DataConversion.openPercentageToLevel(percentType.doubleValue());
double level = DataConversion.openPercentageToLevel(percentCommand.doubleValue());
this.updateServiceState(this.shutterControlService, new ShutterControlServiceState(level));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.openhab.core.thing.ChannelUID;
import org.openhab.core.thing.Thing;
import org.openhab.core.types.Command;
import org.openhab.core.util.ColorUtil;

/**
* Handler for smart light bulbs connected via Zigbee, e.g. Ledvance Smart+ bulbs
Expand Down Expand Up @@ -68,38 +69,38 @@ public void handleCommand(ChannelUID channelUID, Command command) {

switch (channelUID.getId()) {
case CHANNEL_POWER_SWITCH:
if (command instanceof OnOffType) {
updateBinarySwitchState((OnOffType) command);
if (command instanceof OnOffType onOffCommand) {
updateBinarySwitchState(onOffCommand);
}
break;
case CHANNEL_BRIGHTNESS:
if (command instanceof PercentType) {
updateMultiLevelSwitchState((PercentType) command);
if (command instanceof PercentType percentCommand) {
updateMultiLevelSwitchState(percentCommand);
}
break;
case CHANNEL_COLOR:
if (command instanceof HSBType) {
updateColorState((HSBType) command);
if (command instanceof HSBType hsbCommand) {
updateColorState(hsbCommand);
}
break;
}
}

private void updateBinarySwitchState(OnOffType command) {
private void updateBinarySwitchState(OnOffType onOffCommand) {
BinarySwitchServiceState serviceState = new BinarySwitchServiceState();
serviceState.on = command == OnOffType.ON;
serviceState.on = onOffCommand == OnOffType.ON;
this.updateServiceState(binarySwitchService, serviceState);
}

private void updateMultiLevelSwitchState(PercentType command) {
private void updateMultiLevelSwitchState(PercentType percentCommand) {
MultiLevelSwitchServiceState serviceState = new MultiLevelSwitchServiceState();
serviceState.level = command.intValue();
serviceState.level = percentCommand.intValue();
this.updateServiceState(multiLevelSwitchService, serviceState);
}

private void updateColorState(HSBType command) {
private void updateColorState(HSBType hsbCommand) {
HSBColorActuatorServiceState serviceState = new HSBColorActuatorServiceState();
serviceState.rgb = command.getRGB();
serviceState.rgb = ColorUtil.hsbTosRgb(hsbCommand);
this.updateServiceState(hsbColorActuatorService, serviceState);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ public State toState() {
switch (this) {
case LOW_BATTERY:
return new DecimalType(10);
case CRITICAL_LOW:
case CRITICALLY_LOW_BATTERY:
case CRITICAL_LOW, CRITICALLY_LOW_BATTERY:
return new DecimalType(1);
case NOT_AVAILABLE:
return UnDefType.UNDEF;
Expand All @@ -108,9 +107,7 @@ public State toState() {
*/
public OnOffType toLowBatteryState() {
switch (this) {
case LOW_BATTERY:
case CRITICAL_LOW:
case CRITICALLY_LOW_BATTERY:
case LOW_BATTERY, CRITICAL_LOW, CRITICALLY_LOW_BATTERY:
return OnOffType.ON;
default:
return OnOffType.OFF;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public enum SilentModeState {
MODE_NORMAL,
MODE_SILENT;

public static SilentModeState fromOnOffType(OnOffType onOffType) {
return onOffType == OnOffType.ON ? SilentModeState.MODE_SILENT : SilentModeState.MODE_NORMAL;
public static SilentModeState fromOnOffType(OnOffType onOffCommand) {
return onOffCommand == OnOffType.ON ? SilentModeState.MODE_SILENT : SilentModeState.MODE_NORMAL;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
*
*/
@NonNullByDefault
public class BoschSHCHandlerFactoryTest {
class BoschSHCHandlerFactoryTest {

private @NonNullByDefault({}) BoschSHCHandlerFactory fixture;

Expand All @@ -39,7 +39,7 @@ public void setUp() throws Exception {
}

@Test
public void testSupportsThingType() {
void testSupportsThingType() {
assertTrue(fixture.supportsThingType(BoschSHCBindingConstants.THING_TYPE_SHC));
assertTrue(fixture.supportsThingType(BoschSHCBindingConstants.THING_TYPE_INWALL_SWITCH));
assertTrue(fixture.supportsThingType(BoschSHCBindingConstants.THING_TYPE_TWINGUARD));
Expand All @@ -60,7 +60,7 @@ public void testSupportsThingType() {
}

@Test
public void testCreateHandler() {
void testCreateHandler() {
Thing thing = mock(Thing.class);
when(thing.getThingTypeUID()).thenReturn(BoschSHCBindingConstants.THING_TYPE_SMART_PLUG_COMPACT);
assertTrue(fixture.createHandler(thing) instanceof PlugHandler);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
*
*/
@NonNullByDefault
public class BridgeConfigurationTest {
class BridgeConfigurationTest {

@Test
void testConstructor() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
*
*/
@NonNullByDefault
public class JsonRpcRequestTest {
class JsonRpcRequestTest {

private @NonNullByDefault({}) JsonRpcRequest fixture;

Expand All @@ -35,34 +35,34 @@ protected void setUp() throws Exception {
}

@Test
public void testConstructor() {
void testConstructor() {
assertEquals("2.0", fixture.getJsonrpc());
assertEquals("RE/longPoll", fixture.getMethod());
assertArrayEquals(new String[] { "subscriptionId", "20" }, fixture.getParams());
}

@Test
public void testNoArgConstructor() {
void testNoArgConstructor() {
fixture = new JsonRpcRequest();
assertEquals("", fixture.getJsonrpc());
assertEquals("", fixture.getMethod());
assertArrayEquals(new String[0], fixture.getParams());
}

@Test
public void testSetJsonrpc() {
void testSetJsonrpc() {
fixture.setJsonrpc("test");
assertEquals("test", fixture.getJsonrpc());
}

@Test
public void testSetMethod() {
void testSetMethod() {
fixture.setMethod("RE/subscribe");
assertEquals("RE/subscribe", fixture.getMethod());
}

@Test
public void testSetParams() {
void testSetParams() {
fixture.setParams(new String[] { "com/bosch/sh/remote/*", null });
assertArrayEquals(new String[] { "com/bosch/sh/remote/*", null }, fixture.getParams());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
*/
@NonNullByDefault
@ExtendWith(MockitoExtension.class)
public class LongPollingTest {
class LongPollingTest {

/**
* A dummy implementation of {@link ScheduledFuture}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
* @author David Pace - Initial contribution
*
*/
public class DeviceServiceDataTest {
class DeviceServiceDataTest {

private DeviceServiceData fixture;

Expand All @@ -34,7 +34,7 @@ void beforeEach() {
}

@Test
public void testToString() {
void testToString() {
assertEquals("64-da-a0-02-14-9b state: DeviceServiceData", fixture.toString());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ void testIsValid() {
}

@Test
public void testToString() {
void testToString() {
assertEquals(
"Type device; RootDeviceId: 64-da-a0-02-14-9b; Id: hdm:HomeMaticIP:3014F711A00004953859F31B; Device Service Ids: PowerMeter, PowerSwitch, PowerSwitchProgram, Routing; Manufacturer: BOSCH; Room Id: hz_3; Device Model: PSM; Serial: 3014F711A00004953859F31B; Profile: GENERIC; Name: Coffee Machine; Status: AVAILABLE; Child Device Ids: null ",
fixture.toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
* @author Christian Oeing - Initial contribution
*/
@NonNullByDefault
public class LongPollResultTest {
class LongPollResultTest {

@Test
void noResultsForErrorResult() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
*
*/
@NonNullByDefault
public class CameraHandlerTest extends AbstractBoschSHCDeviceHandlerTest<CameraHandler> {
class CameraHandlerTest extends AbstractBoschSHCDeviceHandlerTest<CameraHandler> {

private @Captor @NonNullByDefault({}) ArgumentCaptor<PrivacyModeServiceState> privacyModeServiceStateCaptor;

Expand All @@ -66,7 +66,7 @@ protected String getDeviceID() {
}

@Test
public void testHandleCommandPrivacyMode()
void testHandleCommandPrivacyMode()
throws InterruptedException, TimeoutException, ExecutionException, BoschSHCException {
getFixture().handleCommand(new ChannelUID(getThing().getUID(), BoschSHCBindingConstants.CHANNEL_PRIVACY_MODE),
OnOffType.ON);
Expand All @@ -84,7 +84,7 @@ public void testHandleCommandPrivacyMode()
}

@Test
public void testHandleCommandCameraNotification()
void testHandleCommandCameraNotification()
throws InterruptedException, TimeoutException, ExecutionException, BoschSHCException {
getFixture().handleCommand(
new ChannelUID(getThing().getUID(), BoschSHCBindingConstants.CHANNEL_CAMERA_NOTIFICATION),
Expand All @@ -104,7 +104,7 @@ public void testHandleCommandCameraNotification()
}

@Test
public void testUpdateChannelsPrivacyModeState() {
void testUpdateChannelsPrivacyModeState() {
JsonElement jsonObject = JsonParser.parseString("{\"@type\":\"privacyModeState\",\"value\":\"ENABLED\"}");
getFixture().processUpdate("PrivacyMode", jsonObject);
verify(getCallback()).stateUpdated(
Expand All @@ -117,7 +117,7 @@ public void testUpdateChannelsPrivacyModeState() {
}

@Test
public void testUpdateChannelsCameraNotificationState() {
void testUpdateChannelsCameraNotificationState() {
JsonElement jsonObject = JsonParser
.parseString("{\"@type\":\"cameraNotificationState\",\"value\":\"ENABLED\"}");
getFixture().processUpdate("CameraNotification", jsonObject);
Expand Down
Loading

0 comments on commit 123982d

Please sign in to comment.