Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
mostly adjusting error handling, especially wrt not being able to find
openHAB's IP address or MAC address

Signed-off-by: Cody Cutrer <[email protected]>
  • Loading branch information
ccutrer committed Dec 4, 2024
1 parent 8f2541c commit f91b729
Show file tree
Hide file tree
Showing 13 changed files with 100 additions and 152 deletions.
36 changes: 21 additions & 15 deletions bundles/org.openhab.binding.wiz/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ Local control must also be enabled with-in the WiZ app in the app settings.
- WiZ Smart Plugs
- Smart fans (with or without a dimmable light)

_Note_ This binding was created for and tested on the full color with tunable white bulbs, however, users have reported success with other bulb types and plugs.
**NOTE:** This binding was created for and tested on the full color with tunable white bulbs, however, users have reported success with other bulb types and plugs.

## Discovery

Expand Down Expand Up @@ -72,19 +72,25 @@ Thing wiz:bulb:lamp "My Lamp" @ "Living Room" [ macAddress="accf23343cxx", ipAdd

## Channels

The binding supports the following channels:

| Channel ID | Item Type | Description | Access |
|-----------------|--------------------|-------------------------------------------------------|--------|
| color | Color | State, intensity, and color of the LEDs | R/W |
| temperature | Dimmer | Color temperature of the bulb | R/W |
| temperature-abs | Number:Temperature | Color temperature of the bulb in Kelvin | R/W |
| brightness | Dimmer | The brightness of the bulb | R/W |
| state | Switch | Whether the bulb is on or off | R/W |
| light-mode | Number | Preset light mode name to run | R/W |
| speed | Dimmer | Speed of the color changes in dynamic light modes | R/W |
| signal-strength | Number | Quality of the bulb's WiFi connection | R |
| last-update | Time | The last time an an update was received from the bulb | R |
The binding supports the following channels. If a device is only a light or only a fan, the channels will
not be in a group.

| Channel ID | Item Type | Description | Access |
|------------------------|----------------------|-------------------------------------------------------|--------|
| light#color | Color | State, intensity, and color of the LEDs | R/W |
| light#temperature | Dimmer | Color temperature of the bulb | R/W |
| light#temperature-abs | Number:Temperature | Color temperature of the bulb in Kelvin | R/W |
| light#brightness | Dimmer | The brightness of the bulb | R/W |
| light#state | Switch | Whether the bulb is on or off | R/W |
| light#light-mode | Number | Preset light mode name to run | R/W |
| light#speed | Dimmer | Speed of the color changes in dynamic light modes | R/W |
| fan#state | Switch | Whether the fan is on or off | R/W |
| fan#speed | Number | Speed of the fan, in arbitrary steps | R/W |
| fan#reverse | Switch | Whether the fan direction is reversed | R/W |
| fan#mode | Number | Special fan modes (Breeze) | R/W |
| device#last-update | Time | The last time an an update was received from the bulb | R |
| device#signal-strength | Number | Quality of the bulb's WiFi connection | R |
| device#rssi | Number:Dimensionless | WiFi Received Signal Strength Indicator (in dB) | R |

## Light Modes

Expand Down Expand Up @@ -137,7 +143,7 @@ Sending a command on the color channel or the temperature channel will cause the
- Fade in/out times are configured in the app.
- Sending too many commands to the bulbs too quickly can cause them to stop responding for a period of time.

## Example item linked to a channel
## Example Item Linked To a Channel

```java
Color LivingRoom_Light_Color "Living Room Lamp" (gLivingroom) {channel="wiz:color-bulb:accf23343cxx:color"}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ public boolean supportsThingType(ThingTypeUID thingTypeUID) {
if (supportsThingType(thing.getThingTypeUID())) {
WizHandler handler;

handler = new WizHandler(thing, this.mediator.getRegistrationParams(), stateDescriptionProvider,
timeZoneProvider);
handler = new WizHandler(thing, mediator, stateDescriptionProvider, timeZoneProvider);

mediator.registerThingAndWizBulbHandler(thing, handler);
return handler;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.binding.wiz.internal.entities.RegistrationRequestParam;
import org.openhab.binding.wiz.internal.entities.SystemConfigResult;
import org.openhab.binding.wiz.internal.entities.WizRequest;
import org.openhab.binding.wiz.internal.entities.WizResponse;
Expand Down Expand Up @@ -116,7 +117,8 @@ protected void startScan() {
String broadcastIp = this.mediator.getNetworkAddressService().getConfiguredBroadcastAddress();
if (broadcastIp != null) {
InetAddress address = InetAddress.getByName(broadcastIp);
WizRequest request = new WizRequest(WizMethodType.Registration, this.mediator.getRegistrationParams());
RegistrationRequestParam registrationRequestParam = mediator.getRegistrationParams();
WizRequest request = new WizRequest(WizMethodType.Registration, registrationRequestParam);
request.setId(0);

byte[] message = this.converter.transformToByteMessage(request);
Expand All @@ -133,6 +135,8 @@ protected void startScan() {
} else {
logger.warn("No broadcast address was configured or discovered! No broadcast sent.");
}
} catch (IllegalStateException e) {
logger.debug("Unable to start background scan: {}", e.getMessage());
} catch (IOException exception) {
logger.debug("Something wrong happened when broadcasting the packet to port {}... msg: {}",
DEFAULT_UDP_PORT, exception.getMessage());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public class WizHandler extends BaseThingHandler {
private final Logger logger = LoggerFactory.getLogger(WizHandler.class);

private @NonNullByDefault({}) WizDeviceConfiguration config;
private RegistrationRequestParam registrationInfo;
private @Nullable RegistrationRequestParam registrationRequestParam;
private int homeId;

private WizSyncState mostRecentState;
Expand All @@ -120,10 +120,14 @@ public class WizHandler extends BaseThingHandler {
* @param thing the thing of the handler.
* @param stateDescriptionProvider A state description provider
*/
public WizHandler(final Thing thing, final RegistrationRequestParam registrationPacket,
public WizHandler(final Thing thing, final WizMediator mediator,
WizStateDescriptionProvider stateDescriptionProvider, TimeZoneProvider timeZoneProvider) {
super(thing);
this.registrationInfo = registrationPacket;
try {
registrationRequestParam = mediator.getRegistrationParams();
} catch (IllegalStateException e) {
registrationRequestParam = null;
}
this.stateDescriptionProvider = stateDescriptionProvider;
this.timeZoneProvider = timeZoneProvider;
this.mostRecentState = new WizSyncState();
Expand Down Expand Up @@ -334,7 +338,7 @@ private void handleFanModeCommand(DecimalType mode) {

private void handleIncreaseDecreaseCommand(boolean isIncrease) {
int oldDimming = mostRecentState.dimming;
int newDimming = 50;
int newDimming;
if (isIncrease) {
newDimming = Math.min(100, oldDimming + 5);
} else {
Expand All @@ -351,7 +355,7 @@ private void handleTemperatureCommand(int temperature) {

private void handleIncreaseDecreaseTemperatureCommand(boolean isIncrease) {
float oldTempPct = colorTempToPercent(mostRecentState.getTemperature()).floatValue();
float newTempPct = 50;
float newTempPct;
if (isIncrease) {
newTempPct = Math.min(100, oldTempPct + 5);
} else {
Expand All @@ -370,7 +374,7 @@ private void handleSpeedCommand(PercentType speed) {

private void handleIncreaseDecreaseSpeedCommand(boolean isIncrease) {
int oldSpeed = mostRecentState.speed;
int newSpeed = 50;
int newSpeed;
if (isIncrease) {
newSpeed = Math.min(100, oldSpeed + 5);
} else {
Expand Down Expand Up @@ -452,11 +456,18 @@ public void initialize() {
fullyInitialized = false;
disposed = false;

// set the thing status to UNKNOWN temporarily
updateStatus(ThingStatus.UNKNOWN);
if (ValidationUtils.isMacNotValid(config.macAddress)) {
if (registrationRequestParam == null) {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
"Unable to determine openHAB's IP or MAC address");
return;
}
if (!ValidationUtils.isMacValid(config.macAddress)) {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, "MAC address is not valid");
return;
}

// set the thing status to UNKNOWN temporarily
updateStatus(ThingStatus.UNKNOWN);
updateDeviceProperties();
initGetStatusAndKeepAliveThread();
fullyInitialized = true;
Expand Down Expand Up @@ -792,7 +803,8 @@ private synchronized void updateDeviceProperties() {
* heart-beats the registration must be re-sent after 30s.
*/
private synchronized void registerWithDevice() {
WizResponse registrationResponse = sendRequestPacket(WizMethodType.Registration, this.registrationInfo);
WizResponse registrationResponse = sendRequestPacket(WizMethodType.Registration,
Objects.requireNonNull(registrationRequestParam));
if (registrationResponse != null) {
if (registrationResponse.getResultSuccess()) {
updateTimestamps();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public interface WizMediator {
* connection.
*
*/
RegistrationRequestParam getRegistrationParams();
RegistrationRequestParam getRegistrationParams() throws IllegalStateException;

/**
* Registers a new {@link Thing} and the corresponding
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.binding.wiz.internal.WizBindingConstants;
import org.openhab.binding.wiz.internal.discovery.WizDiscoveryService;
import org.openhab.binding.wiz.internal.entities.RegistrationRequestParam;
import org.openhab.binding.wiz.internal.entities.WizResponse;
Expand Down Expand Up @@ -175,10 +176,10 @@ private void initMediatorWizBulbUpdateReceiverRunnable() {
try {
logger.trace("Receiver thread is either null, interrupted, or dead.");
WizUpdateReceiverRunnable newReceiver = new WizUpdateReceiverRunnable(this, DEFAULT_LISTENER_UDP_PORT);
Thread newThread = new Thread(newReceiver);
Thread newThread = new Thread(newReceiver,
"OH-binding-" + WizBindingConstants.BINDING_ID + "-ReceiverThread");
newThread.setDaemon(true);
newThread.start();
newThread.setName("wizReceiverThread");
this.receiver = newReceiver;
this.receiverThread = newThread;
} catch (SocketException e) {
Expand All @@ -197,37 +198,22 @@ public Set<Thing> getAllThingsRegistered() {
return this.handlersRegisteredByThing.keySet();
}

private String getMyIpAddress() {
String myIpAddress = networkAddressService.getPrimaryIpv4HostAddress();
if (myIpAddress == null) {
logger.warn("Network interface did not return an IP address!");
return "OHIPAddress";
}
return myIpAddress;
}

private String getMyMacAddress() {
String myMacAddress;
try {
myMacAddress = NetworkUtils.getMyMacAddress(getMyIpAddress());
if (myMacAddress == null) {
logger.warn("No network interface could be found. MAC of openHAB device is unknown.");
return "OHMACAddress";
}
} catch (Exception e) {
logger.warn("MAC Address of openHAB device is invalid.");
return "OHMACAddress";
}
return myMacAddress;
}

/**
* Returns a {@link RegistrationRequestParam} based on the current openHAB
* connection.
*
*
* @throws IllegalStateException
*/
public RegistrationRequestParam getRegistrationParams() {
return new RegistrationRequestParam(getMyIpAddress(), true, 0, getMyMacAddress());
public RegistrationRequestParam getRegistrationParams() throws IllegalStateException {
String ipAddress = networkAddressService.getPrimaryIpv4HostAddress();
String macAddress = null;
if (ipAddress != null) {
macAddress = NetworkUtils.getMacAddress(ipAddress);
}
if (ipAddress == null || macAddress == null) {
throw new IllegalStateException("Unable to determine openHAB's IP and/or MAC address");
}
return new RegistrationRequestParam(ipAddress, true, 0, macAddress);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ private void datagramSocketHealthRoutine() {
this.datagramSocket = dsocket;
logger.trace("Datagram Socket reconnected.");
} catch (SocketException exception) {
logger.warn("Problem creating one new socket on port {}. Error: {}", listeningPort,
logger.debug("Problem creating one new socket on port {}. Error: {}", listeningPort,
exception.getLocalizedMessage());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import java.net.InterfaceAddress;
import java.net.NetworkInterface;
import java.net.SocketException;
import java.rmi.UnknownHostException;
import java.util.Enumeration;

import org.eclipse.jdt.annotation.NonNullByDefault;
Expand All @@ -34,25 +33,24 @@ public final class NetworkUtils {
* Returns the MAC address of the openHAB first network device.
*
* @return The MAC address of the openHAB network device.
* @throws UnknownHostException
* @throws SocketException
*/
public static @Nullable String getMyMacAddress(String matchIP) throws UnknownHostException, SocketException {
String macAddress = null;
Enumeration<NetworkInterface> networks = NetworkInterface.getNetworkInterfaces();
while (networks.hasMoreElements()) {
NetworkInterface network = networks.nextElement();
public static @Nullable String getMacAddress(String matchIP) {
try {
Enumeration<NetworkInterface> networks = NetworkInterface.getNetworkInterfaces();
while (networks.hasMoreElements()) {
NetworkInterface network = networks.nextElement();

if (networkMatchesIP(network, matchIP)) {
byte[] hardwareAddress = network.getHardwareAddress();
if (hardwareAddress == null) {
continue;
if (networkMatchesIP(network, matchIP)) {
byte[] hardwareAddress = network.getHardwareAddress();
if (hardwareAddress == null) {
continue;
}
return convertBytesToMACString(hardwareAddress);
}
macAddress = convertBytesToMACString(hardwareAddress);
break; // Short circuit if we found it
}
} catch (SocketException e) {
}
return macAddress;
return null;
}

private static boolean networkMatchesIP(NetworkInterface network, String ip) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,4 @@ public static boolean isMacValid(final String mac) {
Matcher matcher = VALID_PATTERN.matcher(mac);
return matcher.matches();
}

/**
* Validates if one Mac address is not valid.
*
* @param mac the mac, with or without :
* @return true if is not valid.
*/
public static boolean isMacNotValid(final String macAddress) {
return !isMacValid(macAddress);
}
}
Loading

0 comments on commit f91b729

Please sign in to comment.