From 27d4bcc0098cc0f311f964940ae18bbb3a620ecd Mon Sep 17 00:00:00 2001 From: clinique Date: Wed, 1 Sep 2021 17:40:00 +0200 Subject: [PATCH] Code review #5 Signed-off-by: clinique --- .../org.openhab.binding.freeboxos/README.md | 84 +++++++++---------- .../internal/FreeboxOsBindingConstants.java | 17 ++-- .../internal/api/netshare/SambaConfig.java | 4 +- .../internal/handler/LandlineHandler.java | 8 +- .../internal/handler/RevolutionHandler.java | 3 + .../internal/handler/ServerHandler.java | 12 +-- .../resources/OH-INF/thing/channel-types.xml | 10 +-- .../OH-INF/thing/landline-channel-groups.xml | 22 ++--- .../OH-INF/thing/server-channel-groups.xml | 10 +-- .../OH-INF/thing/server-thing-type.xml | 4 +- 10 files changed, 87 insertions(+), 87 deletions(-) diff --git a/bundles/org.openhab.binding.freeboxos/README.md b/bundles/org.openhab.binding.freeboxos/README.md index 662e002dac70a..019107dd9d023 100644 --- 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/bundles/org.openhab.binding.freeboxos/src/main/java/org/openhab/binding/freeboxos/internal/FreeboxOsBindingConstants.java b/bundles/org.openhab.binding.freeboxos/src/main/java/org/openhab/binding/freeboxos/internal/FreeboxOsBindingConstants.java index a22eb450e6c9c..ed4b983e51f0a 100644 --- a/bundles/org.openhab.binding.freeboxos/src/main/java/org/openhab/binding/freeboxos/internal/FreeboxOsBindingConstants.java +++ b/bundles/org.openhab.binding.freeboxos/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/bundles/org.openhab.binding.freeboxos/src/main/java/org/openhab/binding/freeboxos/internal/api/netshare/SambaConfig.java b/bundles/org.openhab.binding.freeboxos/src/main/java/org/openhab/binding/freeboxos/internal/api/netshare/SambaConfig.java index 80f773d8b33bc..57f49fe407bf1 100644 --- a/bundles/org.openhab.binding.freeboxos/src/main/java/org/openhab/binding/freeboxos/internal/api/netshare/SambaConfig.java +++ b/bundles/org.openhab.binding.freeboxos/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/bundles/org.openhab.binding.freeboxos/src/main/java/org/openhab/binding/freeboxos/internal/handler/LandlineHandler.java b/bundles/org.openhab.binding.freeboxos/src/main/java/org/openhab/binding/freeboxos/internal/handler/LandlineHandler.java index 63a35d575c55c..b3a3fc1a61e36 100644 --- a/bundles/org.openhab.binding.freeboxos/src/main/java/org/openhab/binding/freeboxos/internal/handler/LandlineHandler.java +++ b/bundles/org.openhab.binding.freeboxos/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/bundles/org.openhab.binding.freeboxos/src/main/java/org/openhab/binding/freeboxos/internal/handler/RevolutionHandler.java b/bundles/org.openhab.binding.freeboxos/src/main/java/org/openhab/binding/freeboxos/internal/handler/RevolutionHandler.java index 9b0637e72e54c..fbb121882f454 100644 --- a/bundles/org.openhab.binding.freeboxos/src/main/java/org/openhab/binding/freeboxos/internal/handler/RevolutionHandler.java +++ b/bundles/org.openhab.binding.freeboxos/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/bundles/org.openhab.binding.freeboxos/src/main/java/org/openhab/binding/freeboxos/internal/handler/ServerHandler.java b/bundles/org.openhab.binding.freeboxos/src/main/java/org/openhab/binding/freeboxos/internal/handler/ServerHandler.java index ac25865704e03..60104c1f520ca 100644 --- a/bundles/org.openhab.binding.freeboxos/src/main/java/org/openhab/binding/freeboxos/internal/handler/ServerHandler.java +++ b/bundles/org.openhab.binding.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 { diff --git a/bundles/org.openhab.binding.freeboxos/src/main/resources/OH-INF/thing/channel-types.xml b/bundles/org.openhab.binding.freeboxos/src/main/resources/OH-INF/thing/channel-types.xml index f663225711515..e10fb0d2822eb 100644 --- a/bundles/org.openhab.binding.freeboxos/src/main/resources/OH-INF/thing/channel-types.xml +++ b/bundles/org.openhab.binding.freeboxos/src/main/resources/OH-INF/thing/channel-types.xml @@ -81,14 +81,14 @@ - + Switch Indicates whether Windows File Sharing is enabled Switch - + Switch Indicates whether Windows Printer Sharing is enabled @@ -201,14 +201,14 @@ - + Call Details about call. - + Number:Time Call duration in seconds @@ -223,7 +223,7 @@ - + String Called name for outgoing calls. Caller name for incoming calls diff --git a/bundles/org.openhab.binding.freeboxos/src/main/resources/OH-INF/thing/landline-channel-groups.xml b/bundles/org.openhab.binding.freeboxos/src/main/resources/OH-INF/thing/landline-channel-groups.xml index 1b349530cbc79..ac71df1c8d857 100644 --- a/bundles/org.openhab.binding.freeboxos/src/main/resources/OH-INF/thing/landline-channel-groups.xml +++ b/bundles/org.openhab.binding.freeboxos/src/main/resources/OH-INF/thing/landline-channel-groups.xml @@ -26,17 +26,17 @@ The last accepted phone call - + Caller phone number - + - + - + Caller name @@ -47,14 +47,14 @@ The last missed phone call - + Caller phone number - + - + Caller name @@ -65,17 +65,17 @@ The last outgoing phone call - + Called phone number - + - + - + Name, if known, of the called person diff --git a/bundles/org.openhab.binding.freeboxos/src/main/resources/OH-INF/thing/server-channel-groups.xml b/bundles/org.openhab.binding.freeboxos/src/main/resources/OH-INF/thing/server-channel-groups.xml index d8698544b55fa..2dae1567f1427 100644 --- a/bundles/org.openhab.binding.freeboxos/src/main/resources/OH-INF/thing/server-channel-groups.xml +++ b/bundles/org.openhab.binding.freeboxos/src/main/resources/OH-INF/thing/server-channel-groups.xml @@ -20,16 +20,16 @@ - - - + + - - + + + diff --git a/bundles/org.openhab.binding.freeboxos/src/main/resources/OH-INF/thing/server-thing-type.xml b/bundles/org.openhab.binding.freeboxos/src/main/resources/OH-INF/thing/server-thing-type.xml index 602397ca2c6bf..00b456f886a4b 100644 --- a/bundles/org.openhab.binding.freeboxos/src/main/resources/OH-INF/thing/server-thing-type.xml +++ b/bundles/org.openhab.binding.freeboxos/src/main/resources/OH-INF/thing/server-thing-type.xml @@ -15,7 +15,7 @@ - + @@ -35,7 +35,7 @@ - +