Review of fldigixmlrpc code #467

zcsahok · 2025-02-12T21:36:39Z

A review/rework of Fldigi interworking. It started as I couldn't get the idea behind connerr and connerrcnt leading to reconnection, at least this is stated in the comment (maybe @airween knows it). Then a number of issues popped up, I'm working through them slowly. Hopefully will get some of the issues from #463 fixed.

after a toggle we shall start from a fresh state no extra method is needed as this is done implicitly in changepars refreshbp can be dropped, it's done later in the code

in order to see the current setting

it is used there locally for error reporting

in order to reduce complexity also check DECREFs

to reduce indentation

used for internal error reporting anyway, no need to pass it by the caller

and use xmlrpc_res_free() to free result

…er()

dl1jbe

I stepped through the commits one by one. All are comprehensible. Should be ready as base for further work.

The problem is that the xmlrpc_client_call_server_params() or other function using the "global" client mode return a non-initialized result pointer (a local variable containing garbage) in case of some error situations. This makes impossible to decide whether the result should be free'd or not. The xmlrpc documentation contains no hint on this. The "private" functions on the other can use a pointer passed by the caller, so in this case if the initial NULL is untouched then it hasn't been allocated yet, hence no free'ing is needed.

zcsahok · 2025-02-16T20:01:49Z

Documentation references:
xmlrpc-c: https://xmlrpc-c.sourceforge.io/doc/libxmlrpc.html and https://xmlrpc-c.sourceforge.io/doc/libxmlrpc_client.html
xmlrpc-c sources (not the latest, but usable): https://github.com/ensc/xmlrpc-c
FLdigi xmlrpc: https://xmlrpc-c.sourceforge.io/doc/libxmlrpc_client.html

…XMLRPC

zcsahok · 2025-09-23T19:02:06Z

Now curing my main issue: Fldigi gets blocked when used with TLF.

The reason is that in fldigi_xmlrpc_get_carrier() among other things we set the rig mode and freq. We literally bombard Fldigi with requests, as this function is called each 20 ms. Fldigi duly propagates the requests to the rig, but this high I/O load due to some internal working essentially blocks the decoder/encoder/GUI part of Fldigi making the program irresponsive and also TLF unusable in this combination.

Here is short script to trigger the situation: it sends 300 set_mode requests in about 9 seconds. My setup (Fldigi 4.2.06 + rigctld + rig at 9600 Bd) takes roughly 30 seconds to recover from this flooding. And TLF sends such requests continuously...

import xmlrpc.client
from time import sleep

with xmlrpc.client.ServerProxy("http://localhost:7362/") as fldigi:
    for i in range(300):
        fldigi.rig.set_mode('USB')
        sleep(0.03)

As I'm not sure if this mode/freq setting is actually required (@airween, any idea?), so the workaround is to request the current values and call the setter function only if really needed. Thus at the expense of 2 getter calls we can avoid the unnecessary and trouble making setters.

Will limit the rate of calling Fldigi from the current 50 times per sec (20 ms) to 10 times per sec, as this should be also roughly enough. RTTY at 45 Bd can send less than 10 char/s and for faster modes the practical limit is anyway on the human side.

airween · 2025-09-23T19:25:15Z

Uhm, I'm really sorry, I completely missed this issue (and from mentioned one).

Let me review both soon and reflect all.

airween · 2025-09-23T19:57:56Z

it sends 300 set_mode requests in about 9 seconds

That's very weird. I don't think I had any reason to make this. My RIG uses 4800 baud and the response time for a RIG command is about 200-300 ms. When Tlf uses that it asks the current VFO and the current FREQ, that's two requests. As I remember in background process we use a 500ms sleep, and as I remember I tried to use the same mechanism. But it was years ago...

As I'm not sure if this mode/freq setting is actually required (@airween, any idea?), so the workaround is to request the current values and call the setter function only if really needed. Thus at the expense of 2 getter calls we can avoid the unnecessary and trouble making setters.

Honestly, I don't remember why did I think that mode and freq set was necessary - but may be I found some extra scenario which justified it. Probably we should try your suggestion.

zcsahok · 2025-09-26T19:26:17Z

Background process sleeps for 10 ms between iterations and now only each 10th runs checks fldigi status. Let's see if this is enough.
A quick test with both TLF and fldigi connected to rigctld showed no issues regarding frequency setting. (ok, it was not hardware RTTY but AFSK)

airween · 2025-09-27T18:47:40Z

CQ-WW-RTTY is happening right now, there are many RTTY stations in the air.

I tried to set up my RIG with Fldigi, and I have to tell you that's completely unusable.

As @df7cb explained here, Fldigi's (or the XML-RPC lib) response format depends on used locale. I had to set up the locales for Fldigi, then Tlf could work with that - for a while. After a very few minutes Fldigi completely frozen: I was not able to communicate with it, even the waterfall stopped. I assume the problem was the flood of requests from Tlf (I used Zoli's repository: zcsahok:review_fldigi_xmlrpc, but I tried with the master branch too with same result).

So we definitely need to review - I try to help you guys with this.

zcsahok · 2025-09-27T20:23:28Z

I'll try RTTY (AFSK) tomorrow with my repo and report back.

N0NB · 2025-09-28T01:29:59Z

I would think that polling at a rate faster than once per second is excessive. Maybe I'm missing something.

zcsahok · 2025-09-28T11:32:58Z

All normal here using AFSK RTTY.

airween · 2025-09-28T11:39:10Z

I was not able to transmit signal, just tried to receive.
But after a few minutes, Fldigi just frozen. I mean how long had you been using Fldigi?

zcsahok · 2025-09-28T12:02:40Z

Fldigi was running for an hour when I took the screenshot.
Maybe check the AFC setting, if it's on then that can also trigger the freq setting ping-pong.

zcsahok · 2025-09-28T13:57:38Z

"miniterm" feature (:MIN) works too

- optimize fldigi polling - reuse valid_call_char() from callinput.c

- it now allows slash (/) also; this can be fixed later if it makes problems

zcsahok · 2025-10-08T20:44:32Z

Optimized getting call/exchange data from Fldigi. Also tried to move UI code out of these functions as they are called from the background process.

df7cb · 2025-10-09T12:45:39Z

I would think that polling at a rate faster than once per second is excessive. Maybe I'm missing something.

It has to be much more often so there is no delay after clicking on a call in fldigi before it appears in tlf.

zcsahok · 2025-10-09T17:48:24Z

Just tested it and the delay between clicking on Fldigi and the callsign appearing in TLF is minimal.

zcsahok · 2025-10-09T18:01:28Z

That's it for the moment. Will address the freq control strory in a follow-up PR: the whole fldigi_xmlrpc_get_carrier() function can be probably dropped.

zcsahok added 17 commits February 11, 2025 19:02

reset connerr in fldigi_toggle()

9e7014b

after a toggle we shall start from a fresh state no extra method is needed as this is done implicitly in changepars refreshbp can be dropped, it's done later in the code

wait a bit after :FLDigi in changepars.c

44c23be

in order to see the current setting

move 'xmlrpc_env env' to fldigi_xmlrpc_init()

7e2def2

it is used there locally for error reporting

use xmlrpc_release() for both failed init and cleanup

19954f8

extend xmlrpc_res_init() and use it in fldigi_xmlrpc_query()

38d0ad6

earlier exit from fldigi_xmlrpc_query() if not use_fldigi

8f5d757

extract build_param_array() from fldigi_xmlrpc_query()

2826d03

in order to reduce complexity also check DECREFs

use constant from xmlrpc lib

1c129d3

take early exit on connerr

5b69ddf

to reduce indentation

use own xmlrpc_env in fldigi_xmlrpc_query()

84e11c5

used for internal error reporting anyway, no need to pass it by the caller

return error on failed init

b585d55

allow void call for fldigi_xmlrpc_query()

bac6f18

and use xmlrpc_res_free() to free result

reduce indent in fldigi_get_log_call() and fldigi_get_log_serial_numb…

d9c5f3f

…er()

astyle

623d89c

fix building without fldigixmlrpc

f3e905e

extract parse_call_result()

171ec74

fix compilation error

9052db1

dl1jbe reviewed Feb 14, 2025

View reviewed changes

zcsahok requested a review from airween February 16, 2025 20:02

zcsahok added 5 commits February 16, 2025 20:28

switch to standard XML-RPC format specification

3fd8e8b

make mutexes static

f7fb77d

fldigi: set rig mode/freq only if needed

dfdbf81

define xmlrpc_res only if xmlrpc is in use

e46ce6a

fldigixmlrpc.c: make locals static and protect them with #if HAVE_LIB…

7007cae

…XMLRPC

poll Fldigi less frequently (10 times per sec)

2903556

df7cb mentioned this pull request Sep 23, 2025

TLF and fldigi fighting over frequency in RTTY mode #463

Open

zcsahok added 4 commits October 7, 2025 20:36

rework fldigi_get_log_call

38b41d8

- optimize fldigi polling - reuse valid_call_char() from callinput.c

move UI code out of fldigi_get_log_call()

e2dd991

change thiscall to a boolean flag

46bd671

rework fldigi_get_log_serial_number()

c3f869c

- it now allows slash (/) also; this can be fixed later if it makes problems

change fldigi_set_callfield to bool

b7b98fc

zcsahok marked this pull request as ready for review October 9, 2025 18:01

clarify :miniterm feature in man page

09d6c29

airween approved these changes Oct 26, 2025

View reviewed changes

zcsahok merged commit 1da807d into Tlf:master Oct 26, 2025
2 checks passed

zcsahok deleted the review_fldigixmlrpc branch October 26, 2025 16:42

Uh oh!

Review of fldigixmlrpc code #467

Review of fldigixmlrpc code #467

Uh oh!

Conversation

zcsahok commented Feb 12, 2025

Uh oh!

dl1jbe left a comment

Choose a reason for hiding this comment

Uh oh!

zcsahok commented Feb 16, 2025

Uh oh!

zcsahok commented Sep 23, 2025

Uh oh!

airween commented Sep 23, 2025

Uh oh!

airween commented Sep 23, 2025

Uh oh!

zcsahok commented Sep 26, 2025

Uh oh!

airween commented Sep 27, 2025

Uh oh!

zcsahok commented Sep 27, 2025

Uh oh!

N0NB commented Sep 28, 2025

Uh oh!

zcsahok commented Sep 28, 2025

Uh oh!

airween commented Sep 28, 2025

Uh oh!

zcsahok commented Sep 28, 2025

Uh oh!

zcsahok commented Sep 28, 2025

Uh oh!

zcsahok commented Oct 8, 2025

Uh oh!

df7cb commented Oct 9, 2025

Uh oh!

zcsahok commented Oct 9, 2025

Uh oh!

zcsahok commented Oct 9, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants