Skip to content

Commit

Permalink
[insteon] Code review changes
Browse files Browse the repository at this point in the history
Signed-off-by: jsetton <jeremy.setton@gmail.com>
  • Loading branch information
jsetton committed Sep 18, 2024
1 parent 8062aa4 commit 2eabeb9
Show file tree
Hide file tree
Showing 21 changed files with 487 additions and 548 deletions.
748 changes: 376 additions & 372 deletions bundles/org.openhab.binding.insteon/README.md

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,12 @@ public class InsteonBindingConstants {
public static final String FEATURE_TYPE_OUTLET_SWITCH = "OutletSwitch";
public static final String FEATURE_TYPE_THERMOSTAT_FAN_MODE = "ThermostatFanMode";
public static final String FEATURE_TYPE_THERMOSTAT_SYSTEM_MODE = "ThermostatSystemMode";
public static final String FEATURE_TYPE_THERMOSTAT_COOL_SETPOINT = "ThermostatCoolSetPoint";
public static final String FEATURE_TYPE_THERMOSTAT_HEAT_SETPOINT = "ThermostatHeatSetPoint";
public static final String FEATURE_TYPE_THERMOSTAT_COOL_SETPOINT = "ThermostatCoolSetpoint";
public static final String FEATURE_TYPE_THERMOSTAT_HEAT_SETPOINT = "ThermostatHeatSetpoint";
public static final String FEATURE_TYPE_VENSTAR_FAN_MODE = "VenstarFanMode";
public static final String FEATURE_TYPE_VENSTAR_SYSTEM_MODE = "VenstarSystemMode";
public static final String FEATURE_TYPE_VENSTAR_COOL_SETPOINT = "VenstarCoolSetPoint";
public static final String FEATURE_TYPE_VENSTAR_HEAT_SETPOINT = "VenstarHeatSetPoint";
public static final String FEATURE_TYPE_VENSTAR_COOL_SETPOINT = "VenstarCoolSetpoint";
public static final String FEATURE_TYPE_VENSTAR_HEAT_SETPOINT = "VenstarHeatSetpoint";

// List of specific device types
public static final String DEVICE_TYPE_CLIMATE_CONTROL_VENSTAR_THERMOSTAT = "ClimateControl_VenstarThermostat";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,6 @@ public LegacyDevice() {
lastMsgReceived = System.currentTimeMillis();
}

// --------------------- simple getters -----------------------------

public boolean hasProductKey() {
return productKey != null;
}
Expand Down Expand Up @@ -146,7 +144,6 @@ public boolean hasAnyListeners() {
}
return false;
}
// --------------------- simple setters -----------------------------

public void setStatus(DeviceStatus aI) {
status = aI;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,21 +127,15 @@ public void run() {
long expTime = q.getExpirationTime();
LegacyDevice dev = q.getDevice();
if (expTime > now) {
//
// The head of the queue is not up for processing yet, wait().
//
logger.trace("request queue head: {} must wait for {} msec", dev.getAddress(),
expTime - now);
requestQueues.wait(expTime - now);
//
// note that the wait() can also return because of changes to
// the queue, not just because the time expired!
//
continue;
}
//
// The head of the queue has expired and can be processed!
//
q = requestQueues.poll(); // remove front element
requestQueueHash.remove(dev); // and remove from hash map
long nextExp = dev.processRequestQueue(now);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public abstract class CommandHandler extends FeatureBaseHandler {
/**
* Constructor
*
* @param f The DeviceFeature for which this command was intended.
* @param feature The DeviceFeature for which this command was intended.
* The openHAB commands are issued on an openhab item. The .items files bind
* an openHAB item to a DeviceFeature.
*/
Expand Down Expand Up @@ -123,12 +123,6 @@ public String getId() {
*/
public abstract void handleCommand(InsteonChannelConfiguration config, Command cmd);

//
//
// ---------------- the various command handlers start here -------------------
//
//

/**
* Default command handler
*/
Expand Down Expand Up @@ -2273,10 +2267,6 @@ public boolean canHandle(Command cmd) {

@Override
protected int getCommandCode(Command cmd, byte houseCode) {
//
// I did not have hardware that would respond to the PRESET_DIM codes.
// This code path needs testing.
//
int level = ((PercentType) cmd).intValue() * 32 / 100;
int levelCode = X10_LEVEL_CODES[level % 16];
int cmdCode = level >= 16 ? X10Command.PRESET_DIM_2.code() : X10Command.PRESET_DIM_1.code();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -408,10 +408,6 @@ public static class X10PercentCommandHandler extends LegacyCommandHandler {
@Override
public void handleCommand(InsteonLegacyChannelConfiguration conf, Command cmd, LegacyDevice dev) {
try {
//
// I did not have hardware that would respond to the PRESET_DIM codes.
// This code path needs testing.
//
X10Address address = (X10Address) dev.getAddress();
Msg munit = Msg.makeX10AddressMessage(address); // send unit code
dev.enqueueMessage(munit, feature);
Expand Down Expand Up @@ -715,20 +711,16 @@ public void handleCommand(InsteonLegacyChannelConfiguration conf, Command cmd, L
try {
int dc = transform(((DecimalType) cmd).intValue());
int intFactor = getIntParameter("factor", 1);
//
// determine what level should be, and what field it should be in
//
int ilevel = dc * intFactor;
byte level = (byte) (ilevel > 255 ? 0xFF : ((ilevel < 0) ? 0 : ilevel));
String vfield = getStringParameter("value", "");
if (vfield == null || vfield.isEmpty()) {
logger.warn("{} has no value field specified", nm());
return;
}
//
// figure out what cmd1, cmd2, d1, d2, d3 are supposed to be
// to form a proper message
//
int cmd1 = getIntParameter("cmd1", -1);
if (cmd1 < 0) {
logger.warn("{} has no cmd1 specified!", nm());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,6 @@ boolean isMyDirectAck(Msg msg) {
*/
public abstract boolean dispatch(Msg msg);

//
//
// ------------ implementations of MessageDispatchers start here ------------------
//
//

public static class DefaultDispatcher extends LegacyMessageDispatcher {
DefaultDispatcher(LegacyDeviceFeature f) {
super(f);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -325,12 +325,6 @@ public void setParameters(Map<String, String> map) {
parameters = map;
}

//
//
// ---------------- the various command handler start here -------------------
//
//

public static class DefaultMsgHandler extends LegacyMessageHandler {
public DefaultMsgHandler(LegacyDeviceFeature p) {
super(p);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,6 @@ protected void handleIMMessage(Msg msg, DeviceFeature feature) throws FieldExcep
*/
public abstract boolean dispatch(Msg msg);

//
//
// ------------ implementations of MessageDispatchers start here ------------------
//
//

public static class DefaultDispatcher extends MessageDispatcher {
DefaultDispatcher(DeviceFeature feature) {
super(feature);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,12 +261,6 @@ private boolean matchesParameter(Msg msg, String field, String param) throws Fie
*/
public abstract void handleMessage(byte cmd1, Msg msg);

//
//
// ---------------- the various message handlers start here -------------------
//
//

/**
* Default message handler
*/
Expand Down Expand Up @@ -658,8 +652,8 @@ public void handleMessage(byte cmd1, Msg msg) {
/**
* Button event message handler
*/
public static class ButtonEventHandler extends MessageHandler {
ButtonEventHandler(DeviceFeature feature) {
public static class ButtonEventMsgHandler extends MessageHandler {
ButtonEventMsgHandler(DeviceFeature feature) {
super(feature);
}

Expand Down Expand Up @@ -1853,8 +1847,8 @@ protected Unit<Temperature> getTemperatureUnit() {
/**
* IM button event message handler
*/
public static class IMButtonEventHandler extends MessageHandler {
IMButtonEventHandler(DeviceFeature feature) {
public static class IMButtonEventMsgHandler extends MessageHandler {
IMButtonEventMsgHandler(DeviceFeature feature) {
super(feature);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -324,9 +324,6 @@ public void addChannelConfigs(Map<ChannelUID, Configuration> channelConfigs) {
}

private void display(Console console, Map<String, String> info) {
info.entrySet().stream() //
.sorted(Entry.comparingByKey()) //
.map(Entry::getValue) //
.forEach(console::println);
info.entrySet().stream().sorted(Entry.comparingByKey()).map(Entry::getValue).forEach(console::println);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public class HubIOStream extends IOStream {
private int bufferIdx = -1;

/**
* Constructor for HubIOStream
* Constructor
*
* @param host host name of hub device
* @param port port to connect to
Expand All @@ -67,11 +67,11 @@ public class HubIOStream extends IOStream {
* @param pollInterval hub poll interval (in milliseconds)
* @param scheduler the scheduler
*/
public HubIOStream(String host, int port, String user, String pass, int pollInterval,
public HubIOStream(String host, int port, String username, String password, int pollInterval,
ScheduledExecutorService scheduler) {
this.host = host;
this.port = port;
this.auth = Base64.getEncoder().encodeToString((user + ":" + pass).getBytes(StandardCharsets.UTF_8));
this.auth = Base64.getEncoder().encodeToString((username + ":" + password).getBytes(StandardCharsets.UTF_8));
this.pollInterval = pollInterval;
this.scheduler = scheduler;
}
Expand Down Expand Up @@ -206,10 +206,8 @@ private synchronized void poll() throws IOException {
if (logger.isTraceEnabled()) {
logger.trace("poll: {}", buffer);
}
//
// The Hub maintains a ring buffer where the last two digits (in hex!) represent
// the position of the last byte read.
//
String data = buffer.substring(0, buffer.length() - 2); // pure data w/o index pointer

int nIdx = -1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,10 @@ enum ReplyType {
/**
* Constructor
*
* @param devName the name of the port, i.e. '/dev/insteon'
* @param d The Driver object that manages this port
* @param config the network bridge config
* @param driver the driver that manages this port
* @param serialPortManager the serial port manager
* @param scheduler the scheduler
*/
public LegacyPort(InsteonLegacyNetworkConfiguration config, LegacyDriver driver,
SerialPortManager serialPortManager, ScheduledExecutorService scheduler) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,6 @@ public class MsgDefinition {
MsgDefinition() {
}

/*
* Copy constructor, needed to make a copy of a message
*
* @param m the definition to copy
*/
MsgDefinition(MsgDefinition m) {
fields = new HashMap<>(m.fields);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,18 +204,18 @@ channel-type.insteon.carbonMonoxideAlarm.label = Carbon Monoxide Alarm
channel-type.insteon.carbonMonoxideAlarm.description = Carbon Monoxide Alarm
channel-type.insteon.contact.label = Contact
channel-type.insteon.contact.description = Contact State
channel-type.insteon.coolSetPoint.label = Cool Set Point
channel-type.insteon.coolSetPoint.description = Cool Set Point
channel-type.insteon.coolSetpoint.label = Cool Setpoint
channel-type.insteon.coolSetpoint.description = Cool Setpoint
channel-type.insteon.daylight.label = Daylight
channel-type.insteon.daylight.description = Daylight State
channel-type.insteon.dimmer.label = Dimmer
channel-type.insteon.dimmer.description = Dimmer
channel-type.insteon.energyOffset.label = Energy Set Point Offset
channel-type.insteon.energyOffset.description = Energy Set Point Offset
channel-type.insteon.energyOffset.label = Energy Setpoint Offset
channel-type.insteon.energyOffset.description = Energy Setpoint Offset
channel-type.insteon.energySaving.label = Energy Saving
channel-type.insteon.energySaving.description = Energy Saving
channel-type.insteon.energyUsage.label = Energy Usage
channel-type.insteon.energyUsage.description = Energy Usage in Kilowatt Hour
channel-type.insteon.energyUsage.description = Energy Usage
channel-type.insteon.error.label = Error
channel-type.insteon.error.description = Error
channel-type.insteon.fanMode.label = Fan Mode
Expand All @@ -230,12 +230,14 @@ channel-type.insteon.fanSpeed.state.option.MEDIUM = Medium
channel-type.insteon.fanSpeed.state.option.HIGH = High
channel-type.insteon.fanState.label = Fan State
channel-type.insteon.fanState.description = Fan State
channel-type.insteon.fastOnOff.label = Fast On/Off
channel-type.insteon.fastOnOff.description = Fast On/Off (command only)
channel-type.insteon.heartbeatInterval.label = Heartbeat Interval
channel-type.insteon.heartbeatInterval.description = Heartbeat Interval
channel-type.insteon.heartbeatOnOff.label = Heartbeat On/Off
channel-type.insteon.heartbeatOnOff.description = Heartbeat On/Off
channel-type.insteon.heatSetPoint.label = Heat Set Point
channel-type.insteon.heatSetPoint.description = Heat Set Point
channel-type.insteon.heartbeatOnOff.label = Heartbeat Enabled
channel-type.insteon.heartbeatOnOff.description = Heartbeat Enabled
channel-type.insteon.heatSetpoint.label = Heat Setpoint
channel-type.insteon.heatSetpoint.description = Heat Setpoint
channel-type.insteon.humidity.label = Humidity
channel-type.insteon.humidity.description = Current Humidity
channel-type.insteon.humidityControl.label = Humidity Control
Expand All @@ -253,12 +255,12 @@ channel-type.insteon.leak.label = Leak
channel-type.insteon.leak.description = Leak Detected
channel-type.insteon.ledBrightness.label = LED Brightness
channel-type.insteon.ledBrightness.description = LED Brightness
channel-type.insteon.ledOnOff.label = LED On/Off
channel-type.insteon.ledOnOff.description = LED On/Off
channel-type.insteon.ledOnOff.label = LED Enabled
channel-type.insteon.ledOnOff.description = LED Enabled
channel-type.insteon.ledTraffic.label = LED Traffic
channel-type.insteon.ledTraffic.description = LED Blink on Traffic
channel-type.insteon.lightLevel.label = Light Level
channel-type.insteon.lightLevel.description = Light Level
channel-type.insteon.lightSensitivity.label = Light Sensitivity
channel-type.insteon.lightSensitivity.description = Light Sensitivity
channel-type.insteon.load.label = Load
channel-type.insteon.load.description = Load State
channel-type.insteon.loadSense.label = Load Sense
Expand All @@ -271,6 +273,8 @@ channel-type.insteon.lock.label = Lock
channel-type.insteon.lock.description = Lock
channel-type.insteon.lowBattery.label = Low Battery
channel-type.insteon.lowBattery.description = Low Battery Alert
channel-type.insteon.manualChange.label = Manual Change
channel-type.insteon.manualChange.description = Manual Change (command only)
channel-type.insteon.momentaryDuration.label = Momentary Duration
channel-type.insteon.momentaryDuration.description = Momentary Duration
channel-type.insteon.monitorMode.label = Monitor Mode
Expand All @@ -289,7 +293,7 @@ channel-type.insteon.outletBottom.description = Outlet Bottom
channel-type.insteon.outletTop.label = Outlet Top
channel-type.insteon.outletTop.description = Outlet Top
channel-type.insteon.powerUsage.label = Power Usage
channel-type.insteon.powerUsage.description = Power Usage in Watt
channel-type.insteon.powerUsage.description = Power Usage
channel-type.insteon.program1.label = Program 1
channel-type.insteon.program1.description = Program 1
channel-type.insteon.program2.label = Program 2
Expand Down Expand Up @@ -320,12 +324,8 @@ channel-type.insteon.reverseDirection.label = Reverse Direction
channel-type.insteon.reverseDirection.description = Reverse Motor Direction
channel-type.insteon.rollershutter.label = Rollershutter
channel-type.insteon.rollershutter.description = Rollershutter
channel-type.insteon.sceneFastOnOff.label = Scene Fast On/Off
channel-type.insteon.sceneFastOnOff.description = Scene Fast On/Off (command only)
channel-type.insteon.sceneManualChange.label = Scene Manual Change
channel-type.insteon.sceneManualChange.description = Scene Manual Change (command only)
channel-type.insteon.sceneOnOff.label = Scene On/Off
channel-type.insteon.sceneOnOff.description = Scene On/Off
channel-type.insteon.scene.label = Scene
channel-type.insteon.scene.description = Scene
channel-type.insteon.siren.label = Siren
channel-type.insteon.siren.description = Siren
channel-type.insteon.smokeAlarm.label = Smoke Alarm
Expand Down Expand Up @@ -465,7 +465,7 @@ channel-type.insteon.legacyButtonF.label = Button F
channel-type.insteon.legacyButtonG.label = Button G
channel-type.insteon.legacyButtonH.label = Button H
channel-type.insteon.legacyContact.label = Contact
channel-type.insteon.legacyCoolSetPoint.label = Cool Set Point
channel-type.insteon.legacyCoolSetPoint.label = Cool Setpoint
channel-type.insteon.legacyDimmer.label = Dimmer
channel-type.insteon.legacyFan.label = Fan
channel-type.insteon.legacyFanMode.label = Fan Mode
Expand All @@ -478,7 +478,7 @@ channel-type.insteon.legacyFastOnOffButtonE.label = Fast On/Off Button E
channel-type.insteon.legacyFastOnOffButtonF.label = Fast On/Off Button F
channel-type.insteon.legacyFastOnOffButtonG.label = Fast On/Off Button G
channel-type.insteon.legacyFastOnOffButtonH.label = Fast On/Off Button H
channel-type.insteon.legacyHeatSetPoint.label = Heat Set Point
channel-type.insteon.legacyHeatSetPoint.label = Heat Setpoint
channel-type.insteon.legacyHumidity.label = Humidity
channel-type.insteon.legacyHumidityHigh.label = Humidity High
channel-type.insteon.legacyHumidityLow.label = Humidity Low
Expand Down
Loading

0 comments on commit 2eabeb9

Please sign in to comment.