Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add freq to lanentry #445

Merged
merged 5 commits into from
Oct 14, 2024
Merged

Add freq to lanentry #445

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fce1faf
Fix lan_send of LOGENTRY and FREQMSG
dl1jbe Oct 3, 2024
63f0b3a
Accept and record frequency data from LAN log entries
dl1jbe Oct 3, 2024
fec881e
Cleanup
dl1jbe Oct 11, 2024
99f24f4
change prototype
dl1jbe Oct 8, 2024
ff1e4e0
switch to g_strchomp
dl1jbe Oct 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/background_process.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ void *background_process(void *ptr) {
if (lan_message[0] == '\0') {

if (lan_recv() >= 0) {
lan_message[strlen(lan_message) - 1] = '\0';
g_strchomp(lan_message);
}
}

Expand Down
15 changes: 7 additions & 8 deletions src/lancode.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ char lan_message[256];
//--------------------------------------
int bc_socket_descriptor[MAXNODES];
ssize_t bc_sendto_rc;
int cl_send_inhibit = 0;
bool cl_send_inhibit = false;
struct sockaddr_in bc_address[MAXNODES];
/* host names and UDP ports to send notifications to */
char bc_hostaddress[MAXNODES][16];
Expand Down Expand Up @@ -167,7 +167,7 @@ int lan_recv(void) {
errno = 0; /* clear the error */

if (lan_recv_message[1] == CLUSTERMSG)
cl_send_inhibit = 1; // this node does not send cluster info
cl_send_inhibit = true; // this node does not send cluster info

if (lan_recv_rc > 0)
recv_packets++;
Expand Down Expand Up @@ -269,23 +269,22 @@ static int lan_send(char *lanbuffer) {

/* ----------------- send lan message ----------*/

int send_lan_message(int opcode, char *message) {
void send_lan_message(int opcode, char *message) {
char sendbuffer[102];

sendbuffer[0] = thisnode;
sendbuffer[1] = opcode;
sendbuffer[2] = '\0';
strncat(sendbuffer, message, 98);
if (opcode == CLUSTERMSG) {
if (cl_send_inhibit == 0) {
if (!cl_send_inhibit) {
strcat(sendbuffer, "\n");
lan_send(sendbuffer);
}
}

if (opcode == LOGENTRY) {
sendbuffer[82] = '\0';

strcat(sendbuffer, "\n");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

lan_send(sendbuffer);
}

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is new line needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

See also above.

lan_send(sendbuffer);
}
if (opcode == INCQSONUM) {
Expand All @@ -324,7 +323,7 @@ int send_lan_message(int opcode, char *message) {
lan_send(sendbuffer);
}

return 0;
return;
}

/* ----------------- send talk message ----------*/
Expand Down
22 changes: 11 additions & 11 deletions src/lancode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,16 @@

#define MAXNODES 8

#define LOGENTRY 49
#define CLUSTERMSG 50
#define TLFSPOT 51
#define TLFMSG 52
#define FREQMSG 53
#define INCQSONUM 54
#define TIMESYNC 55
#define QTCRENTRY 56
#define QTCSENTRY 57
#define QTCFLAG 58
#define LOGENTRY '1'
#define CLUSTERMSG '2'
#define TLFSPOT '3'
#define TLFMSG '4'
#define FREQMSG '5'
#define INCQSONUM '6'
#define TIMESYNC '7'
#define QTCRENTRY '8'
#define QTCSENTRY '9'
#define QTCFLAG ':'

#include <hamlib/rig.h>

Expand All @@ -52,7 +52,7 @@ int lan_recv_close(void);
int lan_recv(void);
int lan_send_init(void);
int lan_send_close(void);
int send_lan_message(int opcode, char *message);
void send_lan_message(int opcode, char *message);
void talk(void);
int send_freq(freq_t freq);
void send_time(void) ;
Expand Down
10 changes: 5 additions & 5 deletions src/log_to_disk.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@

pthread_mutex_t disk_mutex = PTHREAD_MUTEX_INITIALIZER;

char lan_logline[81];
char lan_logline[LOGLINELEN + 1];


/* restart band timer if in wpx and qso on new band */
Expand Down Expand Up @@ -106,10 +106,10 @@ void log_to_disk(int from_lan) {

} else { /* qso from lan */

/* LOGENTRY contains 82 characters (node,command and logline */
g_strlcpy(lan_logline, lan_message + 2, 81);
char *fill = g_strnfill(80 - strlen(lan_logline), ' ');
g_strlcat(lan_logline, fill, 81); /* fill with spaces if needed */
/* LOGENTRY contains max. 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); /* add spaces if no frequency data */

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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. >

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if (lan_message[0] != thisnode)
Expand Down
2 changes: 1 addition & 1 deletion src/makelogline.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ char *makelogline(struct qso_t *qso) {
}
FILL_TO(87);

assert(strlen(logline) == 87);
assert(strlen(logline) == LOGLINELEN - 1);

return logline;
}
Expand Down
8 changes: 4 additions & 4 deletions test/test_lancode.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ void test_send_freq_80(void **state) {

assert_int_equal(sendto_call_count, 1);
assert_non_null(sendto_last_message);
assert_string_equal(sendto_last_message, "A5 3567.9");
assert_string_equal(sendto_last_message, "A5 3567.9\n");
}

void test_send_freq_10(void **state) {
Expand All @@ -45,7 +45,7 @@ void test_send_freq_10(void **state) {

assert_int_equal(sendto_call_count, 1);
assert_non_null(sendto_last_message);
assert_string_equal(sendto_last_message, "A528123.5");
assert_string_equal(sendto_last_message, "A528123.5\n");
}

void test_send_freq_80_notrx(void **state) {
Expand All @@ -57,7 +57,7 @@ void test_send_freq_80_notrx(void **state) {

assert_int_equal(sendto_call_count, 1);
assert_non_null(sendto_last_message);
assert_string_equal(sendto_last_message, "A5 80.0");
assert_string_equal(sendto_last_message, "A5 80.0\n");
}

void test_send_freq_10_notrx(void **state) {
Expand All @@ -69,5 +69,5 @@ void test_send_freq_10_notrx(void **state) {

assert_int_equal(sendto_call_count, 1);
assert_non_null(sendto_last_message);
assert_string_equal(sendto_last_message, "A5 10.0");
assert_string_equal(sendto_last_message, "A5 10.0\n");
}