From 75ddac0bf07a22344d7ac4ff1a98b495299c9feb Mon Sep 17 00:00:00 2001 From: TPJ Schikhof Date: Sun, 22 Dec 2024 19:41:58 +0000 Subject: [PATCH] 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 | 23 ++++- res/values/strings.xml | 7 ++ .../Backends/LanBackend/LanLinkProvider.java | 9 +- src/org/kde/kdeconnect/DeviceHost.kt | 65 +++++++++++++ .../UserInterface/CustomDevicesActivity.java | 97 +++++++++++++------ .../UserInterface/CustomDevicesAdapter.java | 38 ++++++-- 6 files changed, 193 insertions(+), 46 deletions(-) create mode 100644 src/org/kde/kdeconnect/DeviceHost.kt diff --git a/res/layout/custom_device_item.xml b/res/layout/custom_device_item.xml index 90724096..347488da 100644 --- a/res/layout/custom_device_item.xml +++ b/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" /> - + app:layout_constraintStart_toStartOf="parent" + app:layout_constraintTop_toTopOf="parent" + tools:text="192.168.0.1" /> - + + + diff --git a/res/values/strings.xml b/res/values/strings.xml index 2f1cfcf1..56dea421 100644 --- a/res/values/strings.xml +++ b/res/values/strings.xml @@ -581,4 +581,11 @@ SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted Show a silent notification to continue playing on this device after closing media Continue playing + Pinged in %1$d milliseconds + Could not ping device + Pinging… + + Host is invalid. Use a valid hostname, IPv4, or IPv6 + Host already exists in the list + diff --git a/src/org/kde/kdeconnect/Backends/LanBackend/LanLinkProvider.java b/src/org/kde/kdeconnect/Backends/LanBackend/LanLinkProvider.java index d0ceeec9..0158e731 100644 --- a/src/org/kde/kdeconnect/Backends/LanBackend/LanLinkProvider.java +++ b/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 ipStringList = CustomDevicesActivity + List 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 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(); } diff --git a/src/org/kde/kdeconnect/DeviceHost.kt b/src/org/kde/kdeconnect/DeviceHost.kt new file mode 100644 index 00000000..4ed0b7d0 --- /dev/null +++ b/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") + } +} diff --git a/src/org/kde/kdeconnect/UserInterface/CustomDevicesActivity.java b/src/org/kde/kdeconnect/UserInterface/CustomDevicesActivity.java index ea1c8772..d76eda1f 100644 --- a/src/org/kde/kdeconnect/UserInterface/CustomDevicesActivity.java +++ b/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 customDeviceList; + private ArrayList 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 deserializeIpList(String serialized) { - ArrayList ipList = new ArrayList<>(); + private static ArrayList deserializeIpList(String serialized) { + ArrayList 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 getCustomDeviceList(SharedPreferences sharedPreferences) { + public static ArrayList getCustomDeviceList(SharedPreferences sharedPreferences) { String deviceListPrefs = sharedPreferences.getString(KEY_CUSTOM_DEVLIST_PREFERENCE, ""); - - return deserializeIpList(deviceListPrefs); + ArrayList 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; } diff --git a/src/org/kde/kdeconnect/UserInterface/CustomDevicesAdapter.java b/src/org/kde/kdeconnect/UserInterface/CustomDevicesAdapter.java index f448363a..d4359b16 100644 --- a/src/org/kde/kdeconnect/UserInterface/CustomDevicesAdapter.java +++ b/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 { - private ArrayList customDevices; + private ArrayList 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 customDevices) { + void setCustomDevices(ArrayList customDevices) { this.customDevices = customDevices; notifyDataSetChanged(); @@ -51,12 +56,13 @@ public class CustomDevicesAdapter extends RecyclerView.Adapter 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