ui: show pairing timeout and offer retry on failure
Three separate gaps around pairing failures: The discovery phase had no visible time limit, so a pairing attempt that found nothing just sat there until it gave up. Page 0 now carries a progress bar that drains over the 30 seconds Solaar itself passes to discover() and set_lock(). It stops as soon as a device is found. The passkey page deliberately gets no countdown: that timeout lives in the receiver's firmware and is never reported, so any timer shown there would be invented. The failure page ran gettext over a runtime value, which xgettext cannot extract, so no pairing error was ever actually translated. The cause now comes from literals matched exactly against the error labels that can reach the page, which also lets each one say something useful. Bolt's "failed" in particular used to land on "No further details are available about the error"; it now says the sequence was not accepted, and admits that the receiver cannot tell which buttons were pressed rather than guessing at a cause. The raw error token is kept in a small line so bug reports still carry it. Recovering from a failure meant closing the dialog and starting over from the main window, so the page now offers a "Try again" button. It is packed into the page rather than added as an assistant action widget, so the path stays usable wherever the assistant is only duck-typed, and it is only built when a retry callback was supplied. The Bolt instructions also gained the long-press hint the command line already had, and the passkey page a reminder to keep the device on and in range. The labels added to both pages are bounded and centred so that they wrap inside the existing dialog width instead of stretching it.
This commit is contained in:
Daniel Banariba
parent
68946a7f1d
commit
b4a8344533
|
|
@ -63,6 +63,12 @@ Bolt devices and some Lightspeed devices pair by pressing a special pairing butt
|
|||
To pair with a Bolt receiver you have to type a passcode followed by enter
|
||||
or click the left and right buttons in the correct sequence followed by
|
||||
clicking both buttons simultaneously.
|
||||
Solaar shows this sequence as numbered steps, each one indicating which button
|
||||
to press, with the final both-buttons step set apart from the rest.
|
||||
Receivers that report entry progress also advance a highlight as each press is
|
||||
accepted. That highlight only counts presses: the receiver never reports which
|
||||
button was pressed, so Solaar cannot tell you whether a press was the right one,
|
||||
and only reports whether the whole sequence was accepted.
|
||||
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -67,7 +67,7 @@ def pair(window, receiver):
|
|||
assert receiver
|
||||
assert receiver.kind is None
|
||||
|
||||
pair_dialog = pair_window.create(receiver)
|
||||
pair_dialog = pair_window.create(receiver, on_retry=lambda: pair(window, receiver))
|
||||
pair_dialog.set_transient_for(window)
|
||||
pair_dialog.set_destroy_with_parent(True)
|
||||
pair_dialog.set_modal(True)
|
||||
|
|
|
|||
|
|
@ -47,6 +47,7 @@ _STEP_ENTER = "enter"
|
|||
_PASSKEY_PAGE = "_solaar_passkey_page"
|
||||
_PAIRED_ADDRESS = "_solaar_paired_address"
|
||||
_ON_RETRY = "_solaar_on_retry"
|
||||
_COUNTDOWN = "_solaar_countdown"
|
||||
_STEPS = "_solaar_steps"
|
||||
_PROGRESS = "_solaar_progress"
|
||||
|
||||
|
|
@ -62,6 +63,7 @@ _SEPARATOR_GAP = 10
|
|||
_STRIP_MARGIN = 8
|
||||
_STEPS_PER_ROW = 5
|
||||
_FINAL_STEP_SCALE = 1.25
|
||||
_LABEL_WIDTH_CHARS = 40 # keeps the wrapped labels inside the width of the dialog
|
||||
|
||||
|
||||
class GtkSignal(Enum):
|
||||
|
|
@ -71,13 +73,15 @@ class GtkSignal(Enum):
|
|||
DRAW = "draw"
|
||||
|
||||
|
||||
def create(receiver):
|
||||
def create(receiver, on_retry=None):
|
||||
receiver.reset_pairing() # clear out any information on previous pairing
|
||||
title = _("%(receiver_name)s: pair new device") % {"receiver_name": receiver.name}
|
||||
if receiver.receiver_kind == "bolt":
|
||||
text = _("Bolt receivers are only compatible with Bolt devices.")
|
||||
text += "\n\n"
|
||||
text += _("Press a pairing button or key until the pairing light flashes quickly.")
|
||||
text += "\n"
|
||||
text += _("Press and hold the pairing button on the device for about three seconds.")
|
||||
else:
|
||||
if receiver.receiver_kind == "unifying":
|
||||
text = _("Unifying receivers are only compatible with Unifying devices.")
|
||||
|
|
@ -109,12 +113,35 @@ def create(receiver):
|
|||
)
|
||||
text += _("\nCancelling at this point will not use up a pairing.")
|
||||
ok = prepare(receiver)
|
||||
assistant = _create_assistant(receiver, ok, _finish, title, text)
|
||||
assistant = _create_assistant(receiver, ok, _finish, title, text, on_retry)
|
||||
if ok:
|
||||
GLib.timeout_add(_STATUS_CHECK, check_lock_state, assistant, receiver)
|
||||
deadline = time.monotonic() + _PAIRING_TIMEOUT
|
||||
GLib.timeout_add(_STATUS_CHECK, _update_countdown, assistant, receiver, deadline)
|
||||
return assistant
|
||||
|
||||
|
||||
def _update_countdown(assistant, receiver, deadline):
|
||||
"""Counts down the time left to find a device.
|
||||
|
||||
Only this phase gets a countdown: Solaar sets its timeout itself, whereas the
|
||||
passkey entry timeout lives in the receiver's firmware and is never reported,
|
||||
so any timer shown during entry would be made up.
|
||||
"""
|
||||
countdown = getattr(assistant, _COUNTDOWN, None)
|
||||
if countdown is None:
|
||||
return False
|
||||
remaining = deadline - time.monotonic()
|
||||
found = receiver.pairing.device_address or getattr(assistant, _PASSKEY_PAGE, None) is not None
|
||||
if not assistant.is_drawable() or remaining <= 0 or found:
|
||||
countdown.hide()
|
||||
return False
|
||||
seconds = int(math.ceil(remaining))
|
||||
countdown.set_fraction(remaining / _PAIRING_TIMEOUT)
|
||||
countdown.set_text(ngettext("%d second left", "%d seconds left", seconds) % seconds)
|
||||
return True
|
||||
|
||||
|
||||
def prepare(receiver):
|
||||
if receiver.receiver_kind == "bolt":
|
||||
if receiver.discover(timeout=_PAIRING_TIMEOUT):
|
||||
|
|
@ -171,7 +198,7 @@ def _check_lock_state(assistant, receiver, count):
|
|||
def _pairing_failed(assistant, receiver, error):
|
||||
assistant.remove_page(0) # needed to reset the window size
|
||||
logger.debug("%s fail: %s", receiver, error)
|
||||
_create_failure_page(assistant, error)
|
||||
_create_failure_page(assistant, receiver, error)
|
||||
|
||||
|
||||
def _pairing_succeeded(assistant, receiver, device):
|
||||
|
|
@ -472,16 +499,22 @@ def _create_passcode_page(assistant, receiver, passkey):
|
|||
page.pack_start(strip, False, False, 0)
|
||||
status = Gtk.Label(label=_("Waiting for the first click…"))
|
||||
status.set_line_wrap(True)
|
||||
status.set_max_width_chars(_LABEL_WIDTH_CHARS)
|
||||
status.set_halign(Gtk.Align.CENTER)
|
||||
page.pack_start(status, False, False, 0)
|
||||
if steps[-1] == _STEP_BOTH:
|
||||
# the receiver reports that a button was pressed but never which one,
|
||||
# so say so rather than let a counted click look like a checked one
|
||||
honesty = Gtk.Label(label=_("Solaar cannot tell which button was pressed — only that the receiver accepted a click."))
|
||||
honesty.set_line_wrap(True)
|
||||
honesty.set_max_width_chars(_LABEL_WIDTH_CHARS)
|
||||
honesty.set_halign(Gtk.Align.CENTER)
|
||||
honesty.get_style_context().add_class("dim-label")
|
||||
page.pack_start(honesty, False, False, 0)
|
||||
reminder = Gtk.Label(label=_("Keep the device switched on and within range until pairing finishes."))
|
||||
reminder.set_line_wrap(True)
|
||||
reminder.set_max_width_chars(_LABEL_WIDTH_CHARS)
|
||||
reminder.set_halign(Gtk.Align.CENTER)
|
||||
reminder.get_style_context().add_class("dim-label")
|
||||
page.pack_start(reminder, False, False, 0)
|
||||
page.show_all()
|
||||
|
|
@ -522,13 +555,16 @@ def _update_passcode_page(page, receiver):
|
|||
strip.queue_draw()
|
||||
|
||||
|
||||
def _create_assistant(receiver, ok, finish, title, text):
|
||||
def _create_assistant(receiver, ok, finish, title, text, on_retry=None):
|
||||
assistant = Gtk.Assistant()
|
||||
assistant.set_title(title)
|
||||
assistant.set_icon_name("list-add")
|
||||
assistant.set_size_request(400, 240)
|
||||
assistant.set_resizable(False)
|
||||
assistant.set_role("pair-device")
|
||||
# stash the callback before any page is built, so that a failure while preparing
|
||||
# also gets a retry button
|
||||
setattr(assistant, _ON_RETRY, on_retry)
|
||||
if ok:
|
||||
page_intro = _create_page(
|
||||
assistant,
|
||||
|
|
@ -537,13 +573,18 @@ def _create_assistant(receiver, ok, finish, title, text):
|
|||
"preferences-desktop-peripherals",
|
||||
text,
|
||||
)
|
||||
countdown = Gtk.ProgressBar()
|
||||
countdown.set_show_text(True)
|
||||
countdown.set_visible(True)
|
||||
page_intro.pack_end(countdown, False, False, 0)
|
||||
setattr(assistant, _COUNTDOWN, countdown)
|
||||
spinner = Gtk.Spinner()
|
||||
spinner.set_visible(True)
|
||||
spinner.start()
|
||||
page_intro.pack_end(spinner, True, True, 24)
|
||||
assistant.set_page_complete(page_intro, True)
|
||||
else:
|
||||
page_intro = _create_failure_page(assistant, receiver.pairing.error)
|
||||
page_intro = _create_failure_page(assistant, receiver, receiver.pairing.error)
|
||||
assistant.connect(GtkSignal.CANCEL.value, finish, receiver)
|
||||
assistant.connect(GtkSignal.CLOSE.value, finish, receiver)
|
||||
return assistant
|
||||
|
|
@ -578,19 +619,65 @@ def _create_success_page(assistant, device):
|
|||
assistant.commit()
|
||||
|
||||
|
||||
def _create_failure_page(assistant, error) -> None:
|
||||
header = _("Pairing failed") + ": " + _(str(error)) + "."
|
||||
if "timeout" in str(error):
|
||||
text = _("Make sure your device is within range, and has a decent battery charge.")
|
||||
elif str(error) == "device not supported":
|
||||
text = _("A new device was detected, but it is not compatible with this receiver.")
|
||||
elif "many" in str(error):
|
||||
text = _("More paired devices than receiver can support.")
|
||||
else:
|
||||
text = _("No further details are available about the error.")
|
||||
_create_page(assistant, Gtk.AssistantPageType.SUMMARY, header, "dialog-error", text)
|
||||
def _failure_text(label):
|
||||
"""Describes a pairing failure, matching the error labels exactly.
|
||||
|
||||
The cause has to be chosen from literals rather than translated at runtime,
|
||||
because gettext can only translate strings it saw when the catalogs were built.
|
||||
"""
|
||||
if label == "device timeout" or label == "failed to open pairing lock":
|
||||
return _("Make sure your device is within range, and has a decent battery charge.")
|
||||
if label == "device not supported":
|
||||
return _("A new device was detected, but it is not compatible with this receiver.")
|
||||
if label == "too many devices":
|
||||
return _("More paired devices than receiver can support.")
|
||||
if label == "sequence timeout":
|
||||
return _("Sequence entry timed out.") + "\n" + _("Press the pairing button on your device again and retry.")
|
||||
if label == "failed":
|
||||
# the receiver reports nothing beyond this, so do not guess at a cause: a
|
||||
# wrong button, a mistimed press and a wrong sequence all arrive as "failed"
|
||||
return (
|
||||
_("The click sequence was not accepted.")
|
||||
+ "\n"
|
||||
+ _("The receiver only reports that verification failed; it cannot tell which buttons were pressed.")
|
||||
+ "\n"
|
||||
+ _("Press the pairing button on your device again and retry.")
|
||||
)
|
||||
return _("No further details are available about the error.")
|
||||
|
||||
|
||||
def _retry_pairing(_button, assistant, receiver, on_retry):
|
||||
_finish(assistant, receiver)
|
||||
on_retry()
|
||||
|
||||
|
||||
def _create_failure_page(assistant, receiver, error) -> Gtk.VBox:
|
||||
page = _create_page(
|
||||
assistant,
|
||||
Gtk.AssistantPageType.SUMMARY,
|
||||
_("Pairing failed"),
|
||||
"dialog-error",
|
||||
_failure_text(str(error)),
|
||||
)
|
||||
token = Gtk.Label()
|
||||
token.set_markup(f"<small><tt>{GLib.markup_escape_text(str(error))}</tt></small>")
|
||||
token.set_line_wrap(True)
|
||||
token.set_max_width_chars(_LABEL_WIDTH_CHARS)
|
||||
token.set_halign(Gtk.Align.CENTER)
|
||||
token.get_style_context().add_class("dim-label")
|
||||
page.pack_start(token, False, False, 0)
|
||||
on_retry = getattr(assistant, _ON_RETRY, None)
|
||||
if on_retry is not None:
|
||||
# packed into the page rather than added as an assistant action widget, so
|
||||
# that this path keeps working wherever the assistant is only duck-typed
|
||||
retry = Gtk.Button(label=_("Try again"))
|
||||
retry.set_halign(Gtk.Align.CENTER)
|
||||
retry.connect(GtkSignal.CLICKED.value, _retry_pairing, assistant, receiver, on_retry)
|
||||
page.pack_start(retry, False, False, 0)
|
||||
page.show_all()
|
||||
assistant.next_page()
|
||||
assistant.commit()
|
||||
return page
|
||||
|
||||
|
||||
def _create_page(assistant, kind, header=None, icon_name=None, text=None) -> Gtk.VBox:
|
||||
|
|
|
|||
|
|
@ -1,3 +1,5 @@
|
|||
import time
|
||||
|
||||
from dataclasses import dataclass
|
||||
from dataclasses import field
|
||||
from typing import Any
|
||||
|
|
@ -8,6 +10,8 @@ import gi
|
|||
import pytest
|
||||
|
||||
from logitech_receiver import receiver
|
||||
from logitech_receiver.hidpp10_constants import BoltPairingError
|
||||
from logitech_receiver.hidpp10_constants import PairingError
|
||||
from solaar.ui import pair_window
|
||||
|
||||
gi.require_version("Gtk", "3.0")
|
||||
|
|
@ -400,3 +404,150 @@ def test_draw_step_strip_without_steps():
|
|||
surface = cairo.ImageSurface(cairo.FORMAT_ARGB32, 10, 10)
|
||||
|
||||
assert pair_window._draw_step_strip(strip, cairo.Context(surface)) is False
|
||||
|
||||
|
||||
@pytest.mark.skipif(not gtk_init, reason="requires Gtk")
|
||||
@pytest.mark.parametrize(
|
||||
"error",
|
||||
[
|
||||
PairingError.DEVICE_TIMEOUT.label,
|
||||
PairingError.DEVICE_NOT_SUPPORTED.label,
|
||||
PairingError.TOO_MANY_DEVICES.label,
|
||||
PairingError.SEQUENCE_TIMEOUT.label,
|
||||
BoltPairingError.DEVICE_TIMEOUT.label,
|
||||
BoltPairingError.FAILED.label,
|
||||
"discovery did not start",
|
||||
"the pairing lock did not open",
|
||||
"failed to open pairing lock",
|
||||
],
|
||||
)
|
||||
def test_create_failure_page_covers_every_error(error, mocker):
|
||||
spy_create = mocker.spy(pair_window, "_create_page")
|
||||
|
||||
pair_window._pairing_failed(Assistant(True), Receiver("nano", "nano"), error)
|
||||
|
||||
assert spy_create.call_count == 1
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"error",
|
||||
[
|
||||
PairingError.DEVICE_TIMEOUT.label,
|
||||
PairingError.DEVICE_NOT_SUPPORTED.label,
|
||||
PairingError.TOO_MANY_DEVICES.label,
|
||||
PairingError.SEQUENCE_TIMEOUT.label,
|
||||
BoltPairingError.FAILED.label,
|
||||
"failed to open pairing lock",
|
||||
],
|
||||
)
|
||||
def test_failure_text_is_specific(error):
|
||||
"""A protocol error must never fall through to the generic message."""
|
||||
assert pair_window._failure_text(error) != pair_window._failure_text("something unheard of")
|
||||
|
||||
|
||||
def test_failure_text_explains_a_rejected_sequence():
|
||||
"""Bolt reports only that verification failed, so the page must not diagnose a cause."""
|
||||
text = pair_window._failure_text(BoltPairingError.FAILED.label)
|
||||
|
||||
assert "not accepted" in text
|
||||
assert "cannot tell which buttons were pressed" in text
|
||||
|
||||
|
||||
@pytest.mark.skipif(not gtk_init, reason="requires Gtk")
|
||||
def test_failure_page_has_no_retry_button_without_a_callback(mocker):
|
||||
"""The periodic check reaches this path with a duck-typed assistant, which must
|
||||
never be asked for anything beyond the methods it already provides."""
|
||||
spy_create = mocker.spy(pair_window, "_create_page")
|
||||
assistant = Assistant(True)
|
||||
|
||||
pair_window._pairing_failed(assistant, Receiver("nano", "nano"), "failed")
|
||||
|
||||
assert spy_create.call_count == 1
|
||||
assert not any(isinstance(child, Gtk.Button) for child in assistant.pages[0].get_children())
|
||||
|
||||
|
||||
@pytest.mark.skipif(not gtk_init, reason="requires Gtk")
|
||||
def test_failure_page_offers_retry_when_wired():
|
||||
r = Receiver("nano", "nano")
|
||||
retried = []
|
||||
assistant = Gtk.Assistant()
|
||||
setattr(assistant, pair_window._ON_RETRY, lambda: retried.append(True))
|
||||
|
||||
page = pair_window._create_failure_page(assistant, r, "failed")
|
||||
buttons = [child for child in page.get_children() if isinstance(child, Gtk.Button)]
|
||||
|
||||
assert len(buttons) == 1
|
||||
assert buttons[0].get_label() == "Try again"
|
||||
|
||||
buttons[0].clicked()
|
||||
|
||||
assert retried == [True]
|
||||
|
||||
|
||||
def _countdown_assistant(drawable=True):
|
||||
assistant = Assistant(drawable)
|
||||
countdown = Gtk.ProgressBar()
|
||||
setattr(assistant, pair_window._COUNTDOWN, countdown)
|
||||
return assistant, countdown
|
||||
|
||||
|
||||
@pytest.mark.skipif(not gtk_init, reason="requires Gtk")
|
||||
@pytest.mark.parametrize("receiver_kind", ["bolt", "unifying"])
|
||||
def test_create_adds_a_discovery_countdown(receiver_kind):
|
||||
r = Receiver(receiver_kind, receiver_kind, True)
|
||||
|
||||
assistant = pair_window.create(r)
|
||||
|
||||
countdown = getattr(assistant, pair_window._COUNTDOWN, None)
|
||||
assert isinstance(countdown, Gtk.ProgressBar)
|
||||
assert countdown in assistant.get_nth_page(0).get_children()
|
||||
|
||||
|
||||
@pytest.mark.skipif(not gtk_init, reason="requires Gtk")
|
||||
def test_update_countdown_drains_and_stops():
|
||||
r = Receiver("bolt", "bolt", True)
|
||||
assistant, countdown = _countdown_assistant()
|
||||
deadline = time.monotonic() + pair_window._PAIRING_TIMEOUT
|
||||
|
||||
assert pair_window._update_countdown(assistant, r, deadline) is True
|
||||
assert 0 < countdown.get_fraction() <= 1
|
||||
assert countdown.get_text()
|
||||
|
||||
# a device was found, so the discovery timeout no longer applies
|
||||
r.pairing.device_address = b"\x01\x02\x03\x04\x05\x06"
|
||||
assert pair_window._update_countdown(assistant, r, deadline) is False
|
||||
|
||||
|
||||
@pytest.mark.skipif(not gtk_init, reason="requires Gtk")
|
||||
def test_update_countdown_stops_when_the_deadline_passes():
|
||||
assistant, _countdown = _countdown_assistant()
|
||||
|
||||
assert pair_window._update_countdown(assistant, Receiver("bolt", "bolt", True), time.monotonic() - 1) is False
|
||||
|
||||
|
||||
@pytest.mark.skipif(not gtk_init, reason="requires Gtk")
|
||||
def test_update_countdown_stops_when_the_dialog_is_gone():
|
||||
assistant, _countdown = _countdown_assistant(drawable=False)
|
||||
deadline = time.monotonic() + pair_window._PAIRING_TIMEOUT
|
||||
|
||||
assert pair_window._update_countdown(assistant, Receiver("bolt", "bolt", True), deadline) is False
|
||||
|
||||
|
||||
def test_update_countdown_without_a_progress_bar():
|
||||
deadline = time.monotonic() + pair_window._PAIRING_TIMEOUT
|
||||
|
||||
assert pair_window._update_countdown(Assistant(True), Receiver("bolt", "bolt", True), deadline) is False
|
||||
|
||||
|
||||
@pytest.mark.skipif(not gtk_init, reason="requires Gtk")
|
||||
def test_no_countdown_during_passkey_entry():
|
||||
"""The entry timeout lives in the receiver's firmware and is never reported, so
|
||||
showing a countdown there would mean inventing one."""
|
||||
r = Receiver("bolt", "bolt", True, receiver.Pairing(lock_open=True, device_passkey="50", device_authentication=0x02))
|
||||
assistant, _countdown = _countdown_assistant()
|
||||
|
||||
pair_window._check_lock_state(assistant, r, 0)
|
||||
|
||||
assert pair_window._update_countdown(assistant, r, time.monotonic() + pair_window._PAIRING_TIMEOUT) is False
|
||||
page = getattr(assistant, pair_window._PASSKEY_PAGE)
|
||||
assert not any(isinstance(child, Gtk.ProgressBar) for child in page.get_children())
|
||||
|
|
|
|||
Loading…
Reference in New Issue