Code review openhab#5

Signed-off-by: clinique <gael@lhopital.org>
lolodomo · Sep 1, 2021 · 27d4bcc · 27d4bcc
1 parent 350052b
commit 27d4bcc
Show file tree

Hide file tree

Showing 10 changed files with 87 additions and 87 deletions.
diff --git a/bundles/org.openhab.binding.freeboxos/README.md b/bundles/org.openhab.binding.freeboxos/README.md
@@ -51,7 +51,7 @@ The *revolution* or *delta* thing requires the following configuration parameter
 
 | Parameter Label | Parameter ID | Description | Required | Default |
 |------------------|-----------------|--------------------------------------------------------------------------|----------|---------|
-| Refresh Interval | refreshInterval | The refresh interval (seconds) which is used to poll the Freebox Server. | Yes | 30 |
+| Refresh Interval | refreshInterval | The refresh interval (seconds) which is used to poll the Freebox Server. | No  | 30 |
 
 ### Player thing 
 
@@ -115,50 +115,44 @@ Once initialized, the thing will generate all available channels.
 
 The following channels are supported:
 
-| Thing | Channel Type ID | Item Type | Access Mode | Description |
-|---------------|--------------------------|-----------|-------------|---------------------------------------------------------------------------------|
-| revolution | lcd_brightness | Number | RW | Brightness level of the screen in percent |
-| revolution | lcd_orientation | Number | RW | Screen Orientation in degrees (0 or 90 or 180 or 270) |
-| revolution | lcd_forced | Switch | RW | Indicates whether the screen orientation forced |
-| server (*) | uptime | Number | R | Time since last reboot of the Freebox Server |
-| server (*) | restarted | Switch | R | Indicates whether the Freebox server hase restarted during the last poll period |
-| server (*) | wifi_status | Switch | RW | Indicates whether the WiFi network is enabled |
-| server (*) | ftp_status | Switch | RW | Indicates whether the FTP server is enabled |
-| server (*) | airmedia_status | Switch | RW | Indicates whether Air Media is enabled |
-| server (*) | upnpav_status | Switch | RW | Indicates whether UPnP AV is enabled |
-| server (*) | sambafileshare_status | Switch | RW | Indicates whether Window File Sharing is enabled |
-| server (*) | sambaprintershare_status | Switch | RW | Indicates whether Window Printer Sharing is enabled |
-| server (*) | xdsl_status | String | R | Status of the xDSL line |
-| server (*) | ftth_status | Switch | R | Status of the Ftth line |
-| server (*) | line_status | String | R | Status of network line connexion |
-| server (*) | ipv4 | String | R | Public IP Address of the Freebox Server |
-| server (*) | rate_up | Number | R | Current upload rate in byte/s |
-| server (*) | rate_down | Number | R | Current download rate in byte/s |
-| server (*) | bytes_up | Number | R | Total uploaded bytes since last connection |
-| server (*) | bytes_down | Number | R | Total downloaded bytes since last connection |
-| phone | state#onhook | Switch | R | Indicates whether the phone is on hook |
-| phone | state#ringing | Switch | R | Is the phone ringing |
-| phone | any#call_number | String | R | Last call: number |
-| phone | any#call_duration | Number | R | Last call: duration in seconds |
-| phone | any#call_timestamp | DateTime | R | Last call: creation timestamp |
-| phone | any#call_status | String | R | Last call: type (ingoing, outgoing, missed) |
-| phone | any#call_name | String | R | Last call: called name for outgoing calls. Caller name for incoming calls |
-| phone | accepted#call_number | String | R | Last accepted call: number |
-| phone | accepted#call_duration | Number | R | Last accepted call: duration in seconds |
-| phone | accepted#call_timestamp | DateTime | R | Last accepted call: creation timestamp |
-| phone | accepted#call_name | String | R | Last accepted call: caller name |
-| phone | missed#call_number | String | R | Last missed call: number |
-| phone | missed#call_duration | Number | R | Last missed call: duration in seconds |
-| phone | missed#call_timestamp | DateTime | R | Last missed call: creation timestamp |
-| phone | missed#call_name | String | R | Last missed call: caller name |
-| phone | outgoing#call_number | String | R | Last outgoing call: number |
-| phone | outgoing#call_duration | Number | R | Last outgoing call: duration in seconds |
-| phone | outgoing#call_timestamp | DateTime | R | Last outgoing call: creation timestamp |
-| phone | outgoing#call_name | String | R | Last outgoing call: called name |
-| net_device | reachable | Switch | R | Indicates whether the network device is reachable |
-| net_interface | reachable | Switch | R | Indicates whether the network interface is reachable |
-| airplay | playurl | String | W | Play an audio or video media from the given URL |
-| airplay | stop | Switch | W | Stop the media playback |
+| Thing | Channel Type ID | Item Type | Access Mode | Description |
+|---------------|----------------------|-----------|-------------|--------------------------------------------------------------------------------|
+| revolution | lcd_brightness | Number | RW | Brightness level of the screen in percent |
+| revolution | lcd_orientation | Number | RW | Screen Orientation in degrees (0 or 90 or 180 or 270) |
+| revolution | lcd_forced | Switch | RW | Indicates whether the screen orientation forced |
+| server (*) | uptime | Number | R | Time since last reboot of the Freebox Server |
+| server (*) | restarted | Switch | R | Indicates whether the Freebox server has restarted during the last poll period |
+| server (*) | wifi_status | Switch | RW | Indicates whether the WiFi network is enabled |
+| server (*) | ftp_status | Switch | RW | Indicates whether the FTP server is enabled |
+| server (*) | airmedia_status | Switch | RW | Indicates whether Air Media is enabled |
+| server (*) | upnpav_status | Switch | RW | Indicates whether UPnP AV is enabled |
+| server (*) | samba-file-status | Switch | RW | Indicates whether Window File Sharing is enabled |
+| server (*) | samba-printer-status | Switch | RW | Indicates whether Window Printer Sharing is enabled |
+| server (*) | xdsl_status | String | R | Status of the xDSL line |
+| server (*) | ftth_status | Switch | R | Status of the Ftth line |
+| server (*) | line_status | String | R | Status of network line connexion |
+| server (*) | ipv4 | String | R | Public IP Address of the Freebox Server |
+| server (*) | rate_up | Number | R | Current upload rate in byte/s |
+| server (*) | rate_down | Number | R | Current download rate in byte/s |
+| server (*) | bytes_up | Number | R | Total uploaded bytes since last connection |
+| server (*) | bytes_down | Number | R | Total downloaded bytes since last connection |
+| phone | state#onhook | Switch | R | Indicates whether the phone is on hook |
+| phone | state#ringing | Switch | R | Is the phone ringing |
+| phone | accepted#number | Call | R | Last accepted call: number |
+| phone | accepted#duration | Number | R | Last accepted call: duration in seconds |
+| phone | accepted#timestamp | DateTime | R | Last accepted call: creation timestamp |
+| phone | accepted#name | String | R | Last accepted call: caller name |
+| phone | missed#number | Call | R | Last missed call: number |
+| phone | missed#timestamp | DateTime | R | Last missed call: creation timestamp |
+| phone | missed#name | String | R | Last missed call: caller name |
+| phone | outgoing#number | Call | R | Last outgoing call: number |
+| phone | outgoing#duration | Number | R | Last outgoing call: duration in seconds |
+| phone | outgoing#timestamp | DateTime | R | Last outgoing call: creation timestamp |
+| phone | outgoing#name | String | R | Last outgoing call: called name |
+| net_device | reachable | Switch | R | Indicates whether the network device is reachable |
+| net_interface | reachable | Switch | R | Indicates whether the network interface is reachable |
+| airplay | playurl | String | W | Play an audio or video media from the given URL |
+| airplay | stop | Switch | W | Stop the media playback |
 
 (*) : server means *delta* or *revolution*
 

diff --git a/...boxos/src/main/java/org/openhab/binding/freeboxos/internal/FreeboxOsBindingConstants.java b/...boxos/src/main/java/org/openhab/binding/freeboxos/internal/FreeboxOsBindingConstants.java
@@ -57,7 +57,7 @@ public class FreeboxOsBindingConstants {
  public static final String CONNECTION_STATUS = "connection-status";
  public static final String SYS_INFO = "sysinfo";
  public static final String ACTIONS = "actions";
- public static final String SAMBA = "samba";
+ public static final String FILE_SHARING = "file-sharing";
  public static final String PLAYER_ACTIONS = "player-actions";
  public static final String CONNECTIVITY = "connectivity";
  public static final String STATE = "state";
@@ -90,19 +90,22 @@ public class FreeboxOsBindingConstants {
  public static final String PCT_BW_DOWN = "bandwidth-usage-down";
  public static final String ONHOOK = "onhook";
  public static final String RINGING = "ringing";
- public static final String CALL_INFO = "call-info";
- public static final String CALL_DURATION = "call-duration";
- public static final String CALL_TIMESTAMP = "call-timestamp";
- public static final String CALL_NAME = "call-name";
+
  public static final String FTP_STATUS = "ftp-status";
- public static final String SAMBA_FILE_STATUS = "fileshare-status";
- public static final String SAMBA_PRINTER_STATUS = "printershare-status";
+ public static final String SAMBA_FILE_STATUS = "samba-file-status";
+ public static final String SAMBA_PRINTER_STATUS = "samba-printer-status";
  public static final String REACHABLE = "reachable";
  public static final String LAST_SEEN = "last-seen";
  public static final String ALTERNATE_RING = "lcd-forced";
  public static final String DECT_ACTIVE = "dect-active";
  public static final String UPNPAV_STATUS = "upnpav-status";
 
+ // Call channels for groups Accepted, Missed and Outgoing
+ public static final String NUMBER = "number";
+ public static final String DURATION = "duration";
+ public static final String TIMESTAMP = "timestamp";
+ public static final String NAME = "name";
+
  // Freebox player channels
  public static final String AIRMEDIA_STATUS = "airmedia-status";
  public static final String KEY_CODE = "key-code";

diff --git a/...eboxos/src/main/java/org/openhab/binding/freeboxos/internal/api/netshare/SambaConfig.java b/...eboxos/src/main/java/org/openhab/binding/freeboxos/internal/api/netshare/SambaConfig.java
@@ -16,8 +16,8 @@
 import org.eclipse.jdt.annotation.Nullable;
 
 /**
- * The {@link SambaConfig} is the Java class used to map the "SambaConfig"
- * structure used by the Samba configuration API
+ * The {@link SambaConfig} is the Java class used to map answer
+ * returned by the Samba configuration API
  * https://dev.freebox.fr/sdk/os/network_share/#
  *
  * @author Laurent Garnier - Initial contribution

diff --git a/...eeboxos/src/main/java/org/openhab/binding/freeboxos/internal/handler/LandlineHandler.java b/...eeboxos/src/main/java/org/openhab/binding/freeboxos/internal/handler/LandlineHandler.java
@@ -101,14 +101,14 @@ private void updateChannels(CallEntry call) {
  String group = call.getType().name().toLowerCase();
  String phoneNumber = call.getNumber();
 
- ChannelUID id = new ChannelUID(getThing().getUID(), group, CALL_INFO);
+ ChannelUID id = new ChannelUID(getThing().getUID(), group, NUMBER);
  updateState(id, new StringType(call.getNumber()));
- updateChannelDateTimeState(group, CALL_TIMESTAMP, call.getDatetime());
+ updateChannelDateTimeState(group, TIMESTAMP, call.getDatetime());
  if (call.getType() != CallType.MISSED) { // Missed call have no duration by definition
- updateChannelQuantity(group, CALL_DURATION, call.getDuration(), Units.SECOND);
+ updateChannelQuantity(group, DURATION, call.getDuration(), Units.SECOND);
  }
  if (phoneNumber != null && !phoneNumber.equals(call.getName())) {
- updateChannelString(group, CALL_NAME, call.getNumber());
+ updateChannelString(group, NAME, call.getNumber());
  }
  }
 

diff --git a/...boxos/src/main/java/org/openhab/binding/freeboxos/internal/handler/RevolutionHandler.java b/...boxos/src/main/java/org/openhab/binding/freeboxos/internal/handler/RevolutionHandler.java
@@ -26,6 +26,7 @@
 import org.openhab.core.library.types.IncreaseDecreaseType;
 import org.openhab.core.library.types.OnOffType;
 import org.openhab.core.library.types.PercentType;
+import org.openhab.core.library.types.QuantityType;
 import org.openhab.core.thing.ChannelUID;
 import org.openhab.core.thing.Thing;
 import org.openhab.core.types.Command;
@@ -111,6 +112,8 @@ private void setBrightness(Command command) throws FreeboxException {
  changeLcdBrightness(() -> config.getBrightness() + (command == IncreaseDecreaseType.INCREASE ? 1 : -1));
  } else if (command instanceof OnOffType) {
  changeLcdBrightness(() -> command == OnOffType.ON ? 100 : 0);
+ } else if (command instanceof QuantityType) {
+ changeLcdBrightness(() -> ((QuantityType<?>) command).intValue());
  } else if (command instanceof DecimalType || command instanceof PercentType) {
  changeLcdBrightness(() -> ((DecimalType) command).intValue());
  } else {

diff --git a/...freeboxos/src/main/java/org/openhab/binding/freeboxos/internal/handler/ServerHandler.java b/...freeboxos/src/main/java/org/openhab/binding/freeboxos/internal/handler/ServerHandler.java
@@ -59,7 +59,7 @@ protected void internalPoll() throws FreeboxException {
  fetchConnectionStatus();
  fetchUPnPAVConfig();
  fetchSambaConfig();
- updateChannelOnOff(ACTIONS, FTP_STATUS, getApi().getFtpManager().getFtpStatus());
+ updateChannelOnOff(FILE_SHARING, FTP_STATUS, getApi().getFtpManager().getFtpStatus());
  updateChannelOnOff(ACTIONS, WIFI_STATUS, getApi().getWifiManager().getStatus());
  }
 
@@ -108,13 +108,13 @@ public void handleCommand(ChannelUID channelUID, Command command) {
  updateChannelOnOff(ACTIONS, WIFI_STATUS, getApi().getWifiManager().setStatus(enable));
  break;
  case FTP_STATUS:
- updateChannelOnOff(ACTIONS, FTP_STATUS, getApi().getFtpManager().changeFtpStatus(enable));
+ updateChannelOnOff(FILE_SHARING, FTP_STATUS, getApi().getFtpManager().changeFtpStatus(enable));
  break;
  case SAMBA_FILE_STATUS:
- updateChannelOnOff(SAMBA, SAMBA_FILE_STATUS, enableSambaFileShare(enable));
+ updateChannelOnOff(FILE_SHARING, SAMBA_FILE_STATUS, enableSambaFileShare(enable));
  break;
  case SAMBA_PRINTER_STATUS:
- updateChannelOnOff(SAMBA, SAMBA_PRINTER_STATUS, enableSambaPrintShare(enable));
+ updateChannelOnOff(FILE_SHARING, SAMBA_PRINTER_STATUS, enableSambaPrintShare(enable));
  break;
  case UPNPAV_STATUS:
  updateChannelOnOff(ACTIONS, UPNPAV_STATUS, getApi().getuPnPAVManager().changeStatus(enable));
@@ -128,8 +128,8 @@ public void handleCommand(ChannelUID channelUID, Command command) {
 
  private void fetchSambaConfig() throws FreeboxException {
  SambaConfig response = getApi().getNetShareManager().getSambaConfig();
- updateChannelOnOff(SAMBA, SAMBA_FILE_STATUS, response.isFileShareEnabled());
- updateChannelOnOff(SAMBA, SAMBA_PRINTER_STATUS, response.isPrintShareEnabled());
+ updateChannelOnOff(FILE_SHARING, SAMBA_FILE_STATUS, response.isFileShareEnabled());
+ updateChannelOnOff(FILE_SHARING, SAMBA_PRINTER_STATUS, response.isPrintShareEnabled());
  }
 
  private boolean enableSambaFileShare(boolean enable) throws FreeboxException {