Rotator Read fields never update after Engage (timer removed in dfb7b02)

[N6RFM]

Alex,

While testing the most recent version in the repo, I encountered a loss of connectivity to my HamLi controlled, LVB Tracker mediated Yaesu G-5500 az/el rotor controller. This has also been an intermittent issue for me before but I could not understand why. The that end, I used the Claude LLM to help troubleshoot the problem. A summary of what was reported back, and a solution that is now working for me is below.

Best,
Bob
N6RFM

Summary

gtk_rot_ctrl_new() no longer starts the rotator widget's own periodic
display-refresh timer. As a result, the Azimuth/Elevation "Read:"
labels in the Rotator Control window can remain permanently stuck at
--- after clicking Engage, even though the underlying rotctld
connection and hardware communication are completely healthy.

Whether this is visible depends on an unrelated setting (the Cycle
spinbox's saved value), which makes it intermittent and hard to
pin down from user reports alone.

Environment

• Reproduced on current master (9399b12, "Fix spelling error")
  and on every tagged release from 2.4 through 2.6.4
• GTK 3.24.41, GLib 2.80.0, built from source on Ubuntu 24.04
• Also reproduced in a headless sandbox using Hamlib's "Dummy"
  rotator backend, so it is not specific to any particular rotator
  hardware or rotctld model

Steps to reproduce

1. Configure any rotator device (Preferences → Interfaces → Rotators)
   with rotctld connection details.
2. Ensure the rotator's saved Cycle value in its .rot config file
   is exactly 10 (see "Root cause" below for why this specific
   value matters).
3. Open a module's Rotator Control window and click Engage.

Expected: The Azimuth/Elevation "Read:" fields update
periodically with the rotator's actual reported position.

Actual: The fields remain at --- indefinitely. The Engage
button correctly toggles to "Disengage," rotctld logs successful
position queries, and a manual rotctl -m 2 -r <host>:<port> p query
returns correct live values — but the GUI never reflects it.

Disengaging and re-engaging, restarting rotctld, or rebuilding from
a clean checkout does not change the outcome.

Root cause

Commit dfb7b02 ("Closing the rotor
control window while a rotor is engaged no longer crashes Gpredict")
fixed a real double-free/crash-on-close bug, but in doing so removed
the only unconditional call that created the widget's periodic
refresh timer:

--- a/src/gtk-rot-ctrl.c
+++ b/src/gtk-rot-ctrl.c
@@ -1681,9 +1681,6 @@ GtkWidget      *gtk_rot_ctrl_new(GtkSatModule * module)
     gtk_box_pack_start(GTK_BOX(rot_ctrl), table, FALSE, FALSE, 5);
     gtk_container_set_border_width(GTK_CONTAINER(rot_ctrl), 5);

-    rot_ctrl->timerid = g_timeout_add(rot_ctrl->delay,
-                                      rot_ctrl_timeout_cb, rot_ctrl);
-
     if (module->target > 0)
         gtk_rot_ctrl_select_sat(rot_ctrl, module->target);

The satellite module's own periodic loop still calls
gtk_rot_ctrl_update() on the widget every cycle, but this alone is
not sufficient to drive the display — confirmed by explicitly
wiring module->rotctrl to a test-instrumented widget (matching the
production code path in gtk-sat-module-popup.c's rotctrl_cb
exactly) and still observing the labels never update. Without
rot_ctrl->timerid being set at creation, the widget's own refresh
cycle is simply never established.

Why it's intermittent rather than always broken

A separate, unrelated handler happens to recreate the same timer as
a side effect:

static void delay_changed_cb(GtkSpinButton * spin, gpointer data)
{
    GtkRotCtrl *ctrl = GTK_ROT_CTRL(data);
    ctrl->delay = (guint) gtk_spin_button_get_value(spin);
    if (ctrl->conf)
        ctrl->conf->cycle = ctrl->delay;
    if (ctrl->timerid > 0)
        g_source_remove(ctrl->timerid);
    ctrl->timerid = g_timeout_add(ctrl->delay, rot_ctrl_timeout_cb, ctrl);
}

This fires on the Cycle spinbox's "value-changed" signal, which
GTK also emits when the value is set programmatically — as
happens automatically at startup when a saved rotator device is
auto-selected via rot_selected_cb():

gtk_spin_button_set_value(GTK_SPIN_BUTTON(ctrl->cycle_spin), ctrl->conf->cycle);

The Cycle spinbox is constructed with
gtk_spin_button_new_with_range(10, 10000, 10), so its value is 10
before any config is applied. GTK only fires "value-changed" when
the new value differs from the current one — so:

Saved Cycle	Signal fires?	Timer created?	Display works?
10	No	No	Stuck at ---
Anything else	Yes	Yes	Works

DEFAULT_CYCLE_MS (1000) is used whenever a .rot file has no
Cycle key at all — the normal case for a rotor configured fresh
through Preferences, since that dialog doesn't expose a Cycle field.
So the bug mostly surfaces for users who have, at some point, set
the live Cycle field to exactly its minimum value (e.g. chasing the
fastest possible update rate), or copied/adapted a .rot file from
elsewhere with that value already present.

How this was isolated

Manual testing on real GS-232-protocol hardware confirmed rotctld
and the serial link were always healthy, which ruled out everything
except the GUI code itself. To pin down the exact commit, a headless,
fully automated reproduction was built:

• Xvfb virtual display + Hamlib's built-in Dummy rotator backend
  (no physical hardware required)
• A small instrumentation hook that creates the Rotator Control
  widget, engages it programmatically, waits, then reads the
  AzRead/ElRead label text directly and reports pass/fail — no
  GUI clicking or screenshots
• git bisect run against this script between a confirmed-good
  commit and current master

This identified dfb7b02 as the exact point where behavior changed,
and follow-up tests confirmed the Cycle-value mechanism specifically
(rather than just the commit boundary) — including confirming that
toggling Engage/Disengage, which is a natural first troubleshooting
step, cannot fix it, since that code path never touches the timer.

Suggested fix

Restore the timer creation, keeping the ctrl->timerid = 0 reset in
gtk_rot_ctrl_destroy() (added in b74d9bd,
the commit immediately after dfb7b02) as the existing guard against
the original double-free:

rot_ctrl->timerid = g_timeout_add(rot_ctrl->delay,
                                  rot_ctrl_timeout_cb, rot_ctrl);

Verified in the sandboxed reproduction above: with this line restored,
the Read fields update correctly even with Cycle set to 10 — the
exact value that leaves current master permanently broken.

Happy to open a PR with this change if useful.

[N6RFM]

Related: the value-clamping issue reported in #424 uses the same code path (delay_changed_cb) that masks this bug for some users — worth reading together.