Add freq to lanentry #445

dl1jbe · 2024-10-03T15:26:48Z

Fix handling of missing QRG for log entry sent via LAN.

zcsahok · 2024-10-04T19:34:49Z

Frequency is correctly stored, but see the comments.

zcsahok · 2024-10-04T19:36:42Z

src/lancode.c

@@ -284,8 +284,7 @@ int send_lan_message(int opcode, char *message) {
    }

    if (opcode == LOGENTRY) {
-	sendbuffer[82] = '\0';
-
+	strcat(sendbuffer, "\n");


sendbuffer seems to contain a correctly formatted log line. I tested it OK without adding a new line.

The reason for adding the newline is on the receiving end. Lines 137-139 in background_process.c always drop the last character of the received string. That is the normal code used to drop appended newlines from strings.

As all LAN-messages are sent as normal text strings it seems appropriate to terminate any of them with a newline. That also makes the debuglog more readable.

I think we should harmonize all LAN messages to that behavior. That would also allow us to simplify the send_lan_message() code.

At the moment (with the suggested changes) we add a newline to almost all messages. Only exception is send_time() where a space is added to the time string to avoid truncation. That should be change too.

We are processing our own messages, so any preprocessing is at least suspicious. Blindly dropping the last char is even worse. This is a legacy code originating from af43a4d. It must have been a quick, but incorrect fix for some unknown problem.

Logging shall cope with any message and in fact it does this OK in background_process.c:

148 fputs(lan_message, fp); 149 fputs("\n", fp);

So I see no reason adding NL on the sending side.

Yes, the whole code (besides the QTCxxx messages) are ancient code (It is also in the very early 0.9.10 version from 2003). There was neither time nor need to sort it out. But let us do it now.

I do not think that it was a fix - I assume Rein had NL terminated strings in mind when writing these lines. But the whole artificial fixes of line length (lan_loglin[xx] = '\0') looks really like hot fixes, especially as all these xx values are mostly wrong.

In meantime I did some work to harmonize the LAN message format to EVERY time use of at least ONE NL (which would be dropped by the two lines in question) - see last commit. The generated messages sometimes contains two NL (even without my change). So that mess should really be sorted out.

See my second message as comment to your plan below.

Could the removal of last char be made more relaxed using g_strchomp() instead?

zcsahok · 2024-10-04T19:37:08Z

src/lancode.c

@@ -298,7 +297,7 @@ int send_lan_message(int opcode, char *message) {
 	lan_send(sendbuffer);
    }
    if (opcode == FREQMSG) {
-	sendbuffer[10] = '\0';
+	strcat(sendbuffer, "\n");


Is new line needed?

send_freq() formats the QRG as xxxx.y with one decimal. Without the newline the decimal gets dropped.

See also above.

zcsahok · 2024-10-04T19:39:17Z

src/log_to_disk.c

@@ -46,7 +46,7 @@

 pthread_mutex_t disk_mutex = PTHREAD_MUTEX_INITIALIZER;

-char lan_logline[81];
+char lan_logline[89];


LOGLINELEN + 1 ? or would LOGLINELEN suffice?

Hmm, comments say LOGLINELEN (87) includes all logline information including linefeed. That would mean LOGLINELEN+1 for buffer.
But as the newline is already stripped away one character less would be enough.
Anyway we should use symbol instead of the hard coded number.

Err, LOGLINELEN is defined as 88.

Yes, log lines in the log file are 88 chars long including the NL. Internally it's the buffer length is the same as there is a NUL instead of NL.

zcsahok · 2024-10-04T19:41:04Z

src/log_to_disk.c

+	/* LOGENTRY contains 89 characters (node,command and logline */
+	g_strlcpy(lan_logline, lan_message + 2, LOGLINELEN);
+	char *fill = g_strnfill(LOGLINELEN - 1 - strlen(lan_logline), ' ');
+	g_strlcat(lan_logline, fill, LOGLINELEN);    /* fill with spaces if needed */


I believe that this would cut away the NL added in the lan code. It is correct, as NL is appended anyway in store_qso().

As discussed above the newline is already cut off here.

zcsahok · 2024-10-04T19:45:38Z

src/log_to_disk.c

+	/* LOGENTRY contains 89 characters (node,command and logline */
+	g_strlcpy(lan_logline, lan_message + 2, LOGLINELEN);
+	char *fill = g_strnfill(LOGLINELEN - 1 - strlen(lan_logline), ' ');
+	g_strlcat(lan_logline, fill, LOGLINELEN);    /* fill with spaces if needed */

 	if (cqwwm2) {	    /* mark as coming from other station */


How about removing this dubious check and have QSOs coming from other nodes marked with an asterisk?

Or even better: align the working with the man page -
CQWW_M2
Put the node ID into the logline (just after the QSO number) to
support Multi/2 operation where the station logging the QSO must
be in the Cabrillo file. This can also be used for M/1 and M/M,
to enable post-contest analysis of the nodes.

That is a good idea. But I would like to check first which other places needs to be changed to make it work and change that afterwards.

Just reread the CQWW rules about logging the second stations contacts.
<The MULTIPLIER SIGNAL will be designated with a “1” in the transmitter column of the Cabrillo log. >

Aha, thanks. Then this will need some work to sort out.

dl1jbe · 2024-10-05T12:06:34Z

Thanks for review. See answers inline.

zcsahok · 2024-10-09T05:33:54Z

How about this plan?

in this PR keep only the minimal changes to get freq into the log and also remove the code for dropping the last char of the LAN message.
document LAN messages in the manual
test to see if messages work as required and fix the arising issues.

I can help with 2) and 3).

dl1jbe · 2024-10-09T12:21:15Z

See the last commit to see a more developed state after harmonizing the sent code.

But, let us discus your plan.

1. in this PR keep only the minimal changes to get freq into the log and also remove the code for dropping the last char of the LAN message.

Ok, rework of the commits can be made easily. I would make a new PR for it. (Maybe not before friday).
But I would still recommend to send ALL message NL terminated in the end (makes debugging more easy). So no need to drop the last char.

2. document LAN messages in the manual

Or maybe better in the code, e.g. in lancode.h. I think the message format is not relevant for normal user.

3. test to see if messages work as required and fix the arising issues.

I can help with 2) and 3).

Fine by me.

zcsahok · 2024-10-09T17:17:21Z

Thanks for the updates. I still see no need for the NL, but can live with it. You can use this PR, no separate one is needed IMO.

Re 2): lancode.h is also a perfect place to document the messages.

- send with terminating newline - drop size restrictions in lan_send_message() - change definition of msg-codes into readable ascii characters

dl1jbe · 2024-10-11T11:30:55Z

That should be step 1. Using g_strchomp is a good idea.

zcsahok · 2024-10-13T19:35:30Z

Thanks for the update.

dl1jbe requested review from airween and zcsahok October 3, 2024 15:27

zcsahok reviewed Oct 4, 2024

View reviewed changes

dl1jbe added 4 commits October 11, 2024 13:00

Fix lan_send of LOGENTRY and FREQMSG

fce1faf

- send with terminating newline - drop size restrictions in lan_send_message() - change definition of msg-codes into readable ascii characters

Accept and record frequency data from LAN log entries

63f0b3a

Cleanup

fec881e

change prototype

99f24f4

dl1jbe force-pushed the add_freq_to_lanentry branch from 1294d1e to 99f24f4 Compare October 11, 2024 11:07

switch to g_strchomp

ff1e4e0

zcsahok self-requested a review October 13, 2024 19:35

zcsahok approved these changes Oct 13, 2024

View reviewed changes

dl1jbe merged commit 678adbc into Tlf:master Oct 14, 2024
2 checks passed

dl1jbe deleted the add_freq_to_lanentry branch October 18, 2024 07:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add freq to lanentry #445

Add freq to lanentry #445

dl1jbe commented Oct 3, 2024

zcsahok commented Oct 4, 2024

zcsahok Oct 4, 2024

dl1jbe Oct 5, 2024

zcsahok Oct 9, 2024

dl1jbe Oct 9, 2024

zcsahok Oct 9, 2024

zcsahok Oct 4, 2024

dl1jbe Oct 5, 2024

zcsahok Oct 4, 2024

dl1jbe Oct 5, 2024

dl1jbe Oct 5, 2024

zcsahok Oct 9, 2024

zcsahok Oct 4, 2024

dl1jbe Oct 5, 2024

zcsahok Oct 4, 2024

dl1jbe Oct 5, 2024

dl1jbe Oct 5, 2024

zcsahok Oct 9, 2024

dl1jbe commented Oct 5, 2024

zcsahok commented Oct 9, 2024

dl1jbe commented Oct 9, 2024

zcsahok commented Oct 9, 2024

dl1jbe commented Oct 11, 2024

zcsahok commented Oct 13, 2024

Add freq to lanentry #445

Add freq to lanentry #445

Conversation

dl1jbe commented Oct 3, 2024

zcsahok commented Oct 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dl1jbe commented Oct 5, 2024

zcsahok commented Oct 9, 2024

dl1jbe commented Oct 9, 2024

zcsahok commented Oct 9, 2024

dl1jbe commented Oct 11, 2024

zcsahok commented Oct 13, 2024