Reworked custom device list

- Solved serialization issue when commas were used
- Validate hosts and show toast message if host is invalid
- Show whether device can be reached over the network
- Show toast message when host already exists
- Code TODO's (including sorting device list)

res/layout/custom_device_item.xml

@@ -24,7 +24,7 @@ SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted
         app:drawableEndCompat="@drawable/ic_delete"
         app:drawableStartCompat="@drawable/ic_delete" />
 
-    <FrameLayout
+    <androidx.constraintlayout.widget.ConstraintLayout
         android:id="@+id/swipeableView"
         android:layout_width="match_parent"
         android:layout_height="match_parent"
@@ -32,17 +32,30 @@ SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted
 
         <TextView
             android:id="@+id/deviceNameOrIP"
-            android:layout_width="match_parent"
+            android:layout_width="wrap_content"
             android:layout_height="wrap_content"
+            android:layout_marginTop="16dp"
             android:background="?android:selectableItemBackground"
             android:gravity="center_vertical"
             android:minHeight="?android:attr/listPreferredItemHeightSmall"
-            android:paddingEnd="?android:attr/listPreferredItemPaddingRight"
             android:paddingStart="?android:attr/listPreferredItemPaddingLeft"
+            android:paddingEnd="?android:attr/listPreferredItemPaddingRight"
             android:textAppearance="?android:attr/textAppearanceListItemSmall"
             android:visibility="visible"
-            tools:text="192.168.0.1"/>
+            app:layout_constraintStart_toStartOf="parent"
+            app:layout_constraintTop_toTopOf="parent"
+            tools:text="192.168.0.1" />
 
-    </FrameLayout>
+        <TextView
+            android:id="@+id/connectionStatus"
+            android:layout_width="wrap_content"
+            android:layout_height="wrap_content"
+            android:layout_marginTop="4dp"
+            android:paddingStart="?android:attr/listPreferredItemPaddingLeft"
+            android:paddingEnd="?android:attr/listPreferredItemPaddingRight"
+            app:layout_constraintStart_toStartOf="parent"
+            app:layout_constraintTop_toTopOf="parent" />
+
+    </androidx.constraintlayout.widget.ConstraintLayout>
 
 </FrameLayout>

res/values/strings.xml

@@ -581,4 +581,11 @@ SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted
     <string name="mpris_keepwatching_settings_summary">Show a silent notification to continue playing on this device after closing media</string>
     <string name="notification_channel_keepwatching">Continue playing</string>
 
+    <string name="ping_result">Pinged in %1$d milliseconds</string>
+    <string name="ping_failed">Could not ping device</string>
+    <string name="ping_in_progress">Pinging…</string>
+
+    <string name="device_host_invalid">Host is invalid. Use a valid hostname, IPv4, or IPv6</string>
+    <string name="device_host_duplicate">Host already exists in the list</string>
+
 </resources>

src/org/kde/kdeconnect/Backends/LanBackend/LanLinkProvider.java

@@ -20,6 +20,7 @@ import org.json.JSONException;
 import org.kde.kdeconnect.Backends.BaseLink;
 import org.kde.kdeconnect.Backends.BaseLinkProvider;
 import org.kde.kdeconnect.Device;
+import org.kde.kdeconnect.DeviceHost;
 import org.kde.kdeconnect.DeviceInfo;
 import org.kde.kdeconnect.Helpers.DeviceHelper;
 import org.kde.kdeconnect.Helpers.SecurityHelpers.SslHelper;
@@ -384,19 +385,19 @@ public class LanLinkProvider extends BaseLinkProvider {
         }
 
         ThreadHelper.execute(() -> {
-            List<String> ipStringList = CustomDevicesActivity
+            List<DeviceHost> hostList = CustomDevicesActivity
                     .getCustomDeviceList(PreferenceManager.getDefaultSharedPreferences(context));
 
             if (TrustedNetworkHelper.isTrustedNetwork(context)) {
-                ipStringList.add("255.255.255.255"); //Default: broadcast.
+                hostList.add(DeviceHost.BROADCAST); //Default: broadcast.
             } else {
                 Log.i("LanLinkProvider", "Current network isn't trusted, not broadcasting");
             }
 
             ArrayList<InetAddress> ipList = new ArrayList<>();
-            for (String ip : ipStringList) {
+            for (DeviceHost host : hostList) {
                 try {
-                    ipList.add(InetAddress.getByName(ip));
+                    ipList.add(InetAddress.getByName(host.toString()));
                 } catch (UnknownHostException e) {
                     e.printStackTrace();
                 }

src/org/kde/kdeconnect/DeviceHost.kt

@@ -0,0 +1,65 @@
+package org.kde.kdeconnect
+
+import org.kde.kdeconnect.Helpers.ThreadHelper
+import java.net.InetAddress
+
+class DeviceHost private constructor(private val host: String) {
+    // Wrapper because Kotlin doesn't allow nested nullability
+    data class PingResult(val latency: Long?)
+
+    /** The amount of milliseconds the ping request took or null it's in progress */
+    var ping: PingResult? = null
+        private set
+
+    /**
+     * Checks if the host can be reached over the network.
+     * @param callback Callback for updating UI elements
+     */
+    fun checkReachable(callback: () -> Unit) {
+        ThreadHelper.execute {
+            try {
+                val address = InetAddress.getByName(this.host)
+                val startTime = System.currentTimeMillis()
+                val pingable = address.isReachable(PING_TIMEOUT)
+                val delayMillis = System.currentTimeMillis() - startTime
+                val pingResult = PingResult(if (pingable) delayMillis else null)
+                ping = pingResult
+            }
+            catch (_: Exception) {
+                ping = PingResult(null)
+            }
+            callback()
+        }
+    }
+
+    init {
+        require(isValidDeviceHost(host)) { "Invalid host" }
+    }
+
+    override fun toString(): String {
+        return this.host
+    }
+
+    companion object {
+        /** Ping timeout */
+        private const val PING_TIMEOUT = 3_000
+        private val hostnameValidityPattern = Regex("^[0-9A-Za-z._-]+$")
+
+        @JvmStatic
+        fun isValidDeviceHost(host: String): Boolean {
+            return hostnameValidityPattern.matches(host)
+        }
+
+        @JvmStatic
+        fun toDeviceHostOrNull(host: String): DeviceHost? {
+            return if (isValidDeviceHost(host)) {
+                DeviceHost(host)
+            } else {
+                null
+            }
+        }
+
+        @JvmField
+        val BROADCAST: DeviceHost = DeviceHost("255.255.255.255")
+    }
+}

src/org/kde/kdeconnect/UserInterface/CustomDevicesActivity.java

@@ -13,6 +13,7 @@ import android.preference.PreferenceManager;
 import android.text.TextUtils;
 import android.view.View;
 import android.widget.TextView;
+import android.widget.Toast;
 
 import androidx.annotation.NonNull;
 import androidx.appcompat.app.AppCompatActivity;
@@ -25,16 +26,16 @@ import com.google.android.material.floatingactionbutton.FloatingActionButton;
 import com.google.android.material.snackbar.BaseTransientBottomBar;
 import com.google.android.material.snackbar.Snackbar;
 
+import org.kde.kdeconnect.DeviceHost;
 import org.kde.kdeconnect_tp.R;
 import org.kde.kdeconnect_tp.databinding.ActivityCustomDevicesBinding;
 
 import java.util.ArrayList;
-import java.util.Collections;
+import java.util.Comparator;
 import java.util.Objects;
 
-//TODO: Require wifi connection so entries can be verified
-//TODO: Resolve to ip address and don't allow unresolvable or duplicates based on ip address
-//TODO: Sort the list
+import kotlin.Unit;
+
 public class CustomDevicesActivity extends AppCompatActivity implements CustomDevicesAdapter.Callback {
     private static final String TAG_ADD_DEVICE_DIALOG = "AddDeviceDialog";
 
@@ -45,7 +46,7 @@ public class CustomDevicesActivity extends AppCompatActivity implements CustomDe
     private RecyclerView recyclerView;
     private TextView emptyListMessage;
 
-    private ArrayList<String> customDeviceList;
+    private ArrayList<DeviceHost> customDeviceList;
     private EditTextAlertDialogFragment addDeviceDialog;
     private SharedPreferences sharedPreferences;
     private CustomDevicesAdapter customDevicesAdapter;
@@ -67,15 +68,19 @@ public class CustomDevicesActivity extends AppCompatActivity implements CustomDe
         Objects.requireNonNull(getSupportActionBar()).setDisplayHomeAsUpEnabled(true);
         getSupportActionBar().setDisplayShowHomeEnabled(true);
 
-        fab.setOnClickListener(v -> showEditTextDialog(""));
+        fab.setOnClickListener(v -> showEditTextDialog(null));
 
         sharedPreferences = PreferenceManager.getDefaultSharedPreferences(this);
 
         customDeviceList = getCustomDeviceList(sharedPreferences);
+        customDeviceList.forEach(host -> host.checkReachable(() -> {
+            runOnUiThread(() -> customDevicesAdapter.notifyDataSetChanged());
+            return Unit.INSTANCE;
+        }));
 
         showEmptyListMessageIfRequired();
 
-        customDevicesAdapter = new CustomDevicesAdapter(this);
+        customDevicesAdapter = new CustomDevicesAdapter(this, getApplicationContext());
         customDevicesAdapter.setCustomDevices(customDeviceList);
 
         recyclerView.setHasFixedSize(true);
@@ -108,7 +113,11 @@ public class CustomDevicesActivity extends AppCompatActivity implements CustomDe
         emptyListMessage.setVisibility(customDeviceList.isEmpty() ? View.VISIBLE : View.GONE);
     }
 
-    private void showEditTextDialog(@NonNull String text) {
+    private void showEditTextDialog(DeviceHost deviceHost) {
+        String text = "";
+        if (deviceHost != null) {
+            text = deviceHost.toString();
+        }
         addDeviceDialog = new EditTextAlertDialogFragment.Builder()
                 .setTitle(R.string.add_device_dialog_title)
                 .setHint(R.string.add_device_hint)
@@ -129,30 +138,37 @@ public class CustomDevicesActivity extends AppCompatActivity implements CustomDe
                 .apply();
     }
 
-    private static ArrayList<String> deserializeIpList(String serialized) {
-        ArrayList<String> ipList = new ArrayList<>();
+    private static ArrayList<DeviceHost> deserializeIpList(String serialized) {
+        ArrayList<DeviceHost> ipList = new ArrayList<>();
 
         if (!serialized.isEmpty()) {
-            Collections.addAll(ipList, serialized.split(IP_DELIM));
+            for (String ip: serialized.split(IP_DELIM)) {
+                DeviceHost deviceHost = DeviceHost.toDeviceHostOrNull(ip);
+                // To prevent crashes when migrating if invalid hosts are present
+                if (deviceHost != null) {
+                    ipList.add(deviceHost);
+                }
+            }
         }
 
         return ipList;
     }
 
-    public static ArrayList<String> getCustomDeviceList(SharedPreferences sharedPreferences) {
+    public static ArrayList<DeviceHost> getCustomDeviceList(SharedPreferences sharedPreferences) {
         String deviceListPrefs = sharedPreferences.getString(KEY_CUSTOM_DEVLIST_PREFERENCE, "");
-
-        return deserializeIpList(deviceListPrefs);
+        ArrayList<DeviceHost> list = deserializeIpList(deviceListPrefs);
+        list.sort(Comparator.comparing(DeviceHost::toString));
+        return list;
     }
 
     @Override
-    public void onCustomDeviceClicked(String customDevice) {
+    public void onCustomDeviceClicked(DeviceHost customDevice) {
         editingDeviceAtPosition = customDeviceList.indexOf(customDevice);
         showEditTextDialog(customDevice);
     }
 
     @Override
-    public void onCustomDeviceDismissed(String customDevice) {
+    public void onCustomDeviceDismissed(DeviceHost customDevice) {
         lastDeletedCustomDevice = new DeletedCustomDevice(customDevice, customDeviceList.indexOf(customDevice));
         customDeviceList.remove(lastDeletedCustomDevice.position);
         customDevicesAdapter.notifyItemRemoved(lastDeletedCustomDevice.position);
@@ -190,19 +206,44 @@ public class CustomDevicesActivity extends AppCompatActivity implements CustomDe
         public void onPositiveButtonClicked() {
             if (addDeviceDialog.editText.getText() != null) {
                 String deviceNameOrIP = addDeviceDialog.editText.getText().toString().trim();
+                DeviceHost host = DeviceHost.toDeviceHostOrNull(deviceNameOrIP);
 
                 // don't add empty string (after trimming)
-                if (!deviceNameOrIP.isEmpty() && !customDeviceList.contains(deviceNameOrIP)) {
-                    if (editingDeviceAtPosition >= 0) {
-                        customDeviceList.set(editingDeviceAtPosition, deviceNameOrIP);
-                        customDevicesAdapter.notifyItemChanged(editingDeviceAtPosition);
-                    } else {
-                        customDeviceList.add(deviceNameOrIP);
-                        customDevicesAdapter.notifyItemInserted(customDeviceList.size() - 1);
-                    }
+                if (host != null) {
+                    if (!customDeviceList.stream().anyMatch(h -> h.toString().equals(host.toString()))) {
+                        if (editingDeviceAtPosition >= 0) {
+                            customDeviceList.set(editingDeviceAtPosition, host);
+                            customDevicesAdapter.notifyItemChanged(editingDeviceAtPosition);
+                            host.checkReachable(() -> {
+                                runOnUiThread(() -> customDevicesAdapter.notifyItemChanged(editingDeviceAtPosition));
+                                return Unit.INSTANCE;
+                            });
+                        }
+                        else {
+                            // Find insertion position to ensure list remains sorted
+                            int pos = 0;
+                            while (customDeviceList.size() - 1 >= pos && customDeviceList.get(pos).toString().compareTo(host.toString()) < 0) {
+                                pos++;
+                            }
+                            final int position = pos;
 
-                    saveList();
-                    showEmptyListMessageIfRequired();
+                            customDeviceList.add(position, host);
+                            customDevicesAdapter.notifyItemInserted(pos);
+                            host.checkReachable(() -> {
+                                runOnUiThread(() -> customDevicesAdapter.notifyItemChanged(position));
+                                return Unit.INSTANCE;
+                            });
+                        }
+
+                        saveList();
+                        showEmptyListMessageIfRequired();
+                    }
+                    else {
+                        Toast.makeText(addDeviceDialog.getContext(), R.string.device_host_duplicate, Toast.LENGTH_SHORT).show();
+                    }
+                }
+                else {
+                    Toast.makeText(addDeviceDialog.getContext(), R.string.device_host_invalid, Toast.LENGTH_SHORT).show();
                 }
             }
         }
@@ -214,10 +255,10 @@ public class CustomDevicesActivity extends AppCompatActivity implements CustomDe
     }
 
     private static class DeletedCustomDevice {
-        @NonNull String hostnameOrIP;
+        @NonNull DeviceHost hostnameOrIP;
         int position;
 
-        DeletedCustomDevice(@NonNull String hostnameOrIP, int position) {
+        DeletedCustomDevice(@NonNull DeviceHost hostnameOrIP, int position) {
             this.hostnameOrIP = hostnameOrIP;
             this.position = position;
         }

src/org/kde/kdeconnect/UserInterface/CustomDevicesAdapter.java

@@ -6,6 +6,7 @@
 
 package org.kde.kdeconnect.UserInterface;
 
+import android.content.Context;
 import android.graphics.Canvas;
 import android.view.LayoutInflater;
 import android.view.View;
@@ -16,21 +17,25 @@ import androidx.annotation.Nullable;
 import androidx.recyclerview.widget.ItemTouchHelper;
 import androidx.recyclerview.widget.RecyclerView;
 
+import org.kde.kdeconnect.DeviceHost;
+import org.kde.kdeconnect_tp.R;
 import org.kde.kdeconnect_tp.databinding.CustomDeviceItemBinding;
 
 import java.util.ArrayList;
 
 public class CustomDevicesAdapter extends RecyclerView.Adapter<CustomDevicesAdapter.ViewHolder> {
-    private ArrayList<String> customDevices;
+    private ArrayList<DeviceHost> customDevices;
     private final Callback callback;
+    private final Context context;
 
-    CustomDevicesAdapter(@NonNull Callback callback) {
+    CustomDevicesAdapter(@NonNull Callback callback, Context context) {
         this.callback = callback;
+        this.context = context;
 
         customDevices = new ArrayList<>();
     }
 
-    void setCustomDevices(ArrayList<String> customDevices) {
+    void setCustomDevices(ArrayList<DeviceHost> customDevices) {
         this.customDevices = customDevices;
 
         notifyDataSetChanged();
@@ -51,12 +56,13 @@ public class CustomDevicesAdapter extends RecyclerView.Adapter<CustomDevicesAdap
         CustomDeviceItemBinding itemBinding =
                 CustomDeviceItemBinding.inflate(LayoutInflater.from(parent.getContext()), parent, false);
 
-        return new ViewHolder(itemBinding);
+        return new ViewHolder(itemBinding, context);
     }
 
     @Override
     public void onBindViewHolder(@NonNull ViewHolder holder, int position) {
-        holder.bind(customDevices.get(position));
+        DeviceHost deviceHost = customDevices.get(position);
+        holder.bind(deviceHost.toString(), deviceHost.getPing());
     }
 
     @Override
@@ -66,15 +72,29 @@ public class CustomDevicesAdapter extends RecyclerView.Adapter<CustomDevicesAdap
 
     class ViewHolder extends RecyclerView.ViewHolder implements SwipeableViewHolder {
         private final CustomDeviceItemBinding itemBinding;
+        private final Context context;
 
-        ViewHolder(@NonNull CustomDeviceItemBinding itemBinding) {
+        ViewHolder(@NonNull CustomDeviceItemBinding itemBinding, Context context) {
             super(itemBinding.getRoot());
             this.itemBinding = itemBinding;
             itemBinding.deviceNameOrIP.setOnClickListener(v -> callback.onCustomDeviceClicked(customDevices.get(getAdapterPosition())));
+            this.context = context;
         }
 
-        void bind(String customDevice) {
+        void bind(String customDevice, DeviceHost.PingResult pingResult) {
             itemBinding.deviceNameOrIP.setText(customDevice);
+            if (pingResult != null) {
+                if (pingResult.getLatency() != null) {
+                    String text = context.getString(R.string.ping_result, pingResult.getLatency());
+                    itemBinding.connectionStatus.setText(text);
+                }
+                else {
+                    itemBinding.connectionStatus.setText(R.string.ping_failed);
+                }
+            }
+            else {
+                itemBinding.connectionStatus.setText(R.string.ping_in_progress);
+            }
         }
 
         @Override
@@ -144,7 +164,7 @@ public class CustomDevicesAdapter extends RecyclerView.Adapter<CustomDevicesAdap
     }
 
     public interface Callback {
-        void onCustomDeviceClicked(String customDevice);
-        void onCustomDeviceDismissed(String customDevice);
+        void onCustomDeviceClicked(DeviceHost customDevice);
+        void onCustomDeviceDismissed(DeviceHost customDevice);
     }
 }