Rescore upon reading the log #292

zcsahok · 2021-11-24T21:53:15Z

Too simple to be true. :-) But let's see.

Here is an example: in the OK-OM contest I didn't set ONE_POINT, so all QSOs were scored with zero points. Mults were configured correctly, so that part is fine.

Added the missing ONE_POINT to the rule file and restarted Tlf: now I've 39 points for the 39 QSOs.

All QSOs were on 80, so as a test I moved the one with section MYJ to 40. I worked MYJ on 80 before, so the net result is that I "earn" it as a new one on 40. This operation was done from within Tlf using :EDIt, no restart was required.

As discussed in #248 the log itself is not updated (currently neither on disk nor in memory), hence the lines still show 0 points in the screenshots above. This could be seen as bug, but the fix must be discussed.

dl1jbe · 2021-11-25T21:44:07Z

Too simple to be true. :-) But let's see.

Yes, that step was obvious. If you compare old code in readcalls() and the code in addcall() carefully you will find some subtle differences in handling of corner cases like arrl contests for the w/ve side. I was not quite sure which behavior is the right one, so I hesitated from the simple change before understanding all implications. But lets go on.

As discussed in #248 the log itself is not updated (currently neither on disk nor in memory), hence the lines still show 0 points in the screenshots above. This could be seen as bug, but the fix must be discussed.

I think it can be made simple (but not without some changes). Let us keep the parsed records ('struct qso_t') and add them to a ptr array. That array would be a complete internal representation of all qsos and comments.

Than we can reconstruct the display line from the record when needed for display. That way it will always be up to date.

As an additional side effect it enables us later to generate the display line in different width (and detail) as soon as we support a variable display width in xterm

Both ideas were the main reason to introduce the 'struct qso'. To keep it simple I just started in spring with only the struct and left out the collection of all structs in the array for later.

As a sidenote, let me ask to keep the commits a little more clean. Having the unrelated changes for station info or scoring in the same commit makes it harder in the review to focus on the real core of the commit.

zcsahok · 2021-11-25T22:06:31Z

re station info: it had to be moved before reading the log (country has to be known) and since it used plain printw's it got partially obscured by readcalls' output (via showmsg with its internal line number tracking), hence the rework.

dl1jbe · 2021-11-26T05:41:43Z

re station info: it had to be moved before reading the log (country has to be known) and since it used plain printw's it got partially obscured by readcalls' output (via showmsg with its internal line number tracking), hence the rework.

Oh, thanks. I missed that point.

src/log_to_disk.c

src/getexchange.c

src/addmult.c

src/readcalls.c

zcsahok · 2021-11-26T17:25:20Z

src/readcalls.c


-    total = 0;
-    nr_qsolines = readcalls(logfile);
    if (qtcdirection > 0) {
 	readqtccalls();


On startup readqtccalls() is called before readcalls(). Hopefully the order is not relevant.

Not quite sure. That needs to be checked.

See issue #310

zcsahok · 2021-11-26T17:26:05Z

test/test_readcalls.c


 // dummy functions
 void readqtccalls() {}
 void shownr(char *msg, int x) {}
-void spaces(int n) {} /* needs more care */
+const char *spaces(int n) { return "";} /* needs more care */


Dummies for getexchange...

zcsahok · 2021-11-26T17:31:44Z

Will add (probably next week) updating the in-memory log using makelogline.
If there were changes the user will be asked whether to update the log on disk.

zcsahok · 2021-12-03T20:02:37Z

Fixed (or at least improved) WPX rescoring. For this the add_pfx call hidden in makelogline had to be moved to addcall where it logically belongs. The somewhat obscure excl_add_veto flag thus could be made local. The condition for WPX has been changed to a hopefully more readable form (if not excluded || not pfxab). I'm not sure if this is actually correct, though.
For the sake of addcall2 tests excl_add_veto has been kept as excl_add_veto2 (will be removed later together with addcall2 as soon we get to the networking part...). Plain addcall tests now check countryscore instead.

src/readcalls.c

zcsahok · 2021-12-05T10:19:23Z

src/readcalls.c


    init_scoring();

    if ((fp = fopen(logfile, "r")) == NULL) {
 	showmsg("Error opening logfile ");
-	refreshp();


Already done in showmsg()

test/test_readcalls.c

zcsahok · 2021-12-05T10:28:45Z

Log lines are re-created using makelogline() and checked against the ones in the file. If a change is detected then once all lines are loaded user is prompted if they want to save (update) the log. Previous log is backed up as <timestamp>_<logfilename>.

Next makelogline() has to be "de-globalized". This will also involve replacing linedata_t with qso_t.

dl1jbe · 2021-12-05T17:49:13Z

The somewhat obscure excl_add_veto flag thus could be made local.

I did some checks for that variable and tried to make some comments about its working - see dl1jbe@842feaf

dl1jbe · 2021-12-05T18:46:22Z

The condition for WPX has been changed to a hopefully more readable form (if not excluded || not pfxab). I'm not sure if this is actually correct, though.

I will try to check the handling in the existing code and compare with older versions. Maybe we can sort it out.

dl1jbe · 2021-12-05T19:08:07Z

src/readcalls.c

+    bool log_changed = false;
+    // array of qso's
+    // FIXME use this instead of qsos[] (make it global and don't free it here)
+    // FIXME also include comments/notes into qso_t


We could just add a 'iscomment' flag and a pointer to the comment string or save some memory by using a union union from the actual qso_t and the comment field pointer.

src/readcalls.c

zcsahok · 2021-12-05T19:26:45Z

Ideally scoring and mult handling shall be independent of whether the QSO originated locally, received from network or read from a file. Therefore any veto logic have to be an internal detail of the processing.

dl1jbe · 2021-12-05T21:03:52Z

We are on the way to it.
I think the best point is that the rules are now conentrated on a single place. In old code we had two places to keep in sync.

zcsahok · 2021-12-06T17:20:58Z

src/makelogline.c

-	    comment[4] = 'X';
-	    comment[5] = '\0';
+	char buf[80];
+	strcpy(buf, qso->comment);


Change a private copy of comment.

src/makelogline.c

src/readcabrillo.c

src/store_qso.c

test/test_cabrillo.c

zcsahok · 2021-12-06T17:32:52Z

Unless there are issues to fix this is it for the moment. I'd let it get used to see if other problems pop up.
There's still a long way ahead: swapping qsos[] with an array of qso_t and moving all QSO-related globals to qso_t are both major changes that have to be done mid term.

zcsahok · 2021-12-20T08:06:52Z

Let me know if you see something open here. If there are none then I'd merge this and also close #248.

dl1jbe

Besides the question about 'freq' if no rig control is active I think all should be well.

Please double check and feel free to merge it in afterwards.

src/addcall.c

dl1jbe · 2021-12-29T17:35:23Z

src/makelogline.c


    /* add points to logline if in contest */
    if (iscontest && !CONTEST_IS(DXPED)) {
-	sprintf(logline4 + 76, "%2d", qso_points);
+	sprintf(logline + 76, "%2d", qso_points);   //FIXME qso->score


Yes, we should keep ALL information for the qso after evaluating it with addcall/addmult.
Question is - should we just extend qso_t or use the parsed data as part of another struct which provides the additional data (points, zone, cty, ...)

dl1jbe · 2021-12-29T17:43:59Z

src/makelogline.c

-    if (trx_control) {
-	snprintf(freq_buff, 8, "%7.1f", freq / 1000.0);
-	strcat(logline4, freq_buff);
+    if (qso->freq > 0) {


If someone use TLF without radio control is freq then set to 0.?
At the moment we do not record a frequency in these case.

trx_control tells whether rig control is currently in use, whereas qso->freq > 0 checks if freq was known at the time of the QSO. Generally this could have been earlier or even currently but on another Tlf node. So relying on trx_control is not correct.

If trx control is disabled while Tlf is running then freq is not set to zero (previous value is kept). Similarly if trx control is enabled then rig connection is not initialized, freq stays zero. IMO both are bugs.

src/makelogline.c

dl1jbe · 2021-12-29T18:33:57Z

src/readcalls.c

+	addcall(qso);
+	score_qso();    //FIXME argument?
+
+	char *logline = makelogline(qso);


After finishing all migration away from qsos[], we do not need to record such things as points, multis or prefixes in the written log.
The we could drop these check for a change of logfile. The file would be the same as long as we do not change some of the qso information (time, call, exchange, band...).
But that leads us again to a discussion of logfile format. We should keep that for later.

dl1jbe · 2021-12-29T18:35:38Z

src/readcalls.c

+	    char *backup = g_strdup_printf("%s_%s", prefix, logfile);
+	    rename(logfile, backup);
+	    // rewrite log
+	    remove(logfile);


There should be no 'logfile' after the rename.

test/test_cabrillo.c

dl1jbe · 2022-01-05T19:41:54Z

Am Tue, 04 Jan 2022 07:01:50 -0800 schrieb Zoltan Csahok ***@***.***>:

@zcsahok commented on this pull request. > /* add freq to end of logline */ - if (trx_control) { `trx_control` tells whether rig control is currently in use, whereas `qso->freq > 0` checks if freq was known at the time of the QSO. Generally this could have been earlier or even currently but on another Tlf node. So relying on `trx_control` is not correct.

Ok, I overlooked the situation wrt other tlf nodes which may report the frequency-

If trx control is disabled while Tlf is running then `freq` is not set to zero (previous value is kept). Similarly if trx control is enabled then rig connection is not initialized, freq stays zero. IMO both are bugs.

Right. Enabling trx_control should try to initialize the rig connection. Needs to be done. Both bugs should be fixed. Will put it on todo list.

…

-- "Do what is needful!" Ursula LeGuin: Earthsea --

rescore upon reading the log

baee0b5

zcsahok linked an issue Nov 24, 2021 that may be closed by this pull request

TLF should calculate points and multis on every start #248

Closed

rescore: handle dupes

2a114b1