Skip to content

Commit

Permalink
app_rpt: Use enum for newkey variables.
Browse files Browse the repository at this point in the history
  • Loading branch information
@mkmer
mkmer authored Jan 15, 2025
1 parent 604328d commit 7660f5c
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 42 deletions.
74 changes: 38 additions & 36 deletions apps/app_rpt.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1677,14 +1677,14 @@ static void handle_link_data(struct rpt *myrpt, struct rpt_link *mylink, char *s
if (!strcmp(tmp, NEWKEYSTR)) {
if ((!mylink->newkey) || mylink->newkeytimer) {
mylink->newkeytimer = 0;
mylink->newkey = 1;
send_old_newkey(mylink->chan);
mylink->newkey = KEYUP_ALLOWED_REDUNDANT;
send_newkey_redundant(mylink->chan);
}
return;
}
if (!strcmp(tmp, NEWKEY1STR)) {
mylink->newkeytimer = 0;
mylink->newkey = 2;
mylink->newkey = KEYUP_NOT_ALLOWED;
return;
}
if (!strncmp(tmp, IAXKEYSTR, strlen(IAXKEYSTR))) {
Expand Down Expand Up @@ -2143,13 +2143,13 @@ static int handle_remote_data(struct rpt *myrpt, char *str)
return 0;
if (!strcmp(tmp, NEWKEYSTR)) {
if (!myrpt->newkey) {
send_old_newkey(myrpt->rxchannel);
myrpt->newkey = 1;
send_newkey_redundant(myrpt->rxchannel);
myrpt->newkey = KEYUP_ALLOWED_REDUNDANT;
}
return 0;
}
if (!strcmp(tmp, NEWKEY1STR)) {
myrpt->newkey = 2;
myrpt->newkey = KEYUP_NOT_ALLOWED;
return 0;
}
if (!strncmp(tmp, IAXKEYSTR, strlen(IAXKEYSTR))) {
Expand Down Expand Up @@ -2272,7 +2272,7 @@ static int attempt_reconnect(struct rpt *myrpt, struct rpt_link *l)
l->connecttime = 0;
l->thisconnected = 0;
l->iaxkey = 0;
l->newkey = 0;
l->newkey = KEYUP_ALLOWED;

cap = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);
if (!cap) {
Expand All @@ -2289,7 +2289,7 @@ static int attempt_reconnect(struct rpt *myrpt, struct rpt_link *l)
l->lastrealrx = 0;
l->rxlingertimer = ((l->iaxkey) ? RX_LINGER_TIME_IAXKEY : RX_LINGER_TIME);
l->newkeytimer = NEWKEYTIME;
l->newkey = 2;
l->newkey = KEYUP_NOT_ALLOWED;
while ((f1 = AST_LIST_REMOVE_HEAD(&l->textq, frame_list)))
ast_frfree(f1);
if (l->chan) {
Expand Down Expand Up @@ -3026,7 +3026,7 @@ static inline void rxunkey_helper(struct rpt *myrpt, struct rpt_link *l)
static inline void periodic_process_links(struct rpt *myrpt, const int elap)
{
struct ast_frame *f;
int x;
int newkeytimer_last;
struct rpt_link *l = myrpt->links.next;
while (l != &myrpt->links) {
int myrx, mymaxct;
Expand All @@ -3043,17 +3043,18 @@ static inline void periodic_process_links(struct rpt *myrpt, const int elap)
l->rxlingertimer = 0;

/* Update the timer, checking if it expired just now. */
x = l->newkeytimer;
newkeytimer_last = l->newkeytimer;
if (l->newkeytimer)
l->newkeytimer -= elap;
if (l->newkeytimer < 0)
l->newkeytimer = 0;

/* Some reverse-engineering comments here from NA debugging issue #46 (inbound calls being keyed when they shouldn't be)
* This if statement executes if the timer just expired.
* This does NOT include cases like in handle_link_data where we set newkeytimer = 0 explicitly + set newkey to 1 or 2 (because then x == 0 here)
* This does NOT include cases like in handle_link_data where we set newkeytimer = 0 explicitly + set newkey
* to KEYUP_ALLOWED_REDUNDANT or KEYUP_NOT_ALLOWED (because then newkeytimer_last == 0 here)
*/
if (x > 0 && !l->newkeytimer) {
if (newkeytimer_last > 0 && !l->newkeytimer) {
/* Issue #46 background:
*
* There is a kind of "handshake" that happens when setting up the IAX2 trunk between two nodes,
Expand All @@ -3078,7 +3079,8 @@ static inline void periodic_process_links(struct rpt *myrpt, const int elap)
* Note that the above depiction separates the TX and RX, but there are only 3 text frames involved.
* In issue #46, 3 text frames are sent, but only 2 are really "received".
* And it so happens that the text frame that B doesn't get from A is exactly the text frame
* that is responsible for setting newkeytimer=0 and newkey=2, i.e. if this doesn't happen, then we'll hit the WARNING case in the below if statement.
* that is responsible for setting newkeytimer=0 and newkey=KEYUP_NOT_ALLOWED, i.e. if this doesn't happen,
* then we'll hit the WARNING case in the below if statement.
*
* Note that all of these comments are from spending hours debugging this issue and reverse-engineering, but at this point I'm pretty confident
* about these parts of the code, even though I'm not Jim Dixon and he didn't comment any of this code originally.
Expand All @@ -3091,16 +3093,16 @@ static inline void periodic_process_links(struct rpt *myrpt, const int elap)
if (l->thisconnected) {
/* We're connected, but haven't received a NEWKEY1STR text frame yet...
* The newkeytimer expired on a connected (~answered?) node, i.e. handle_link_data hasn't yet gotten called
* to set newkeytimer = 0 and newkey to non-zero, i.e. we haven't received a text frame with NEWKEY1STR over the IAX2 channel yet.
* to set newkeytimer = 0 and newkey to KEYUP_NOT_ALLOWED, i.e. we haven't received a text frame with NEWKEY1STR over the IAX2 channel yet.
*/
if (l->newkey == 2) {
if (l->newkey == KEYUP_NOT_ALLOWED) {
/* This can ripple to have consequences down the line, namely we might start writing voice frames
* across the IAX2 link because of this, basically causing us to be transmitting (keyed).
* If this happens, this indicates a problem upstream, and we should emit a warning here
* since undesired behavior will likely ensue.
*/
ast_log(LOG_WARNING, "%p newkeytimer expired on connected node, setting newkey from 2 to 0.\n", l);
l->newkey = 0;
l->newkey = KEYUP_ALLOWED;
}
} else {
/* If not connected yet (maybe a slow link connection?), wait another NEWKEYTIME ms */
Expand All @@ -3112,7 +3114,7 @@ static inline void periodic_process_links(struct rpt *myrpt, const int elap)
if (l->linkmode < 1)
l->linkmode = 1;
}
if ((l->newkey == 2) && l->lastrealrx && (!l->rxlingertimer)) {
if ((l->newkey == KEYUP_NOT_ALLOWED) && l->lastrealrx && (!l->rxlingertimer)) {
rxunkey_helper(myrpt, l);
}

Expand Down Expand Up @@ -3167,7 +3169,7 @@ static inline void periodic_process_links(struct rpt *myrpt, const int elap)
ast_debug(7, "@@@@ node %s sent node string %s to node %s\n", myrpt->name, lstr, l->name);
}
}
if (l->newkey == 1) {
if (l->newkey == KEYUP_ALLOWED_REDUNDANT) {
if ((l->retxtimer += elap) >= REDUNDANT_TX_TIME) {
l->retxtimer = 0;
if (l->chan && l->phonemode == 0) {
Expand Down Expand Up @@ -4294,7 +4296,7 @@ static inline int process_link_channels(struct rpt *myrpt, struct ast_channel *w
*totx = 0;
if (l->phonemode == 0 && l->chan && (l->lasttx != *totx)) {
if (*totx && !l->voterlink) {
if (l->newkey < 2)
if (l->newkey != KEYUP_NOT_ALLOWED)
ast_indicate(l->chan, AST_CONTROL_RADIO_KEY);
} else {
ast_indicate(l->chan, AST_CONTROL_RADIO_UNKEY);
Expand Down Expand Up @@ -4333,7 +4335,7 @@ static inline int process_link_channels(struct rpt *myrpt, struct ast_channel *w

l->rxlingertimer = ((l->iaxkey) ? RX_LINGER_TIME_IAXKEY : RX_LINGER_TIME);

if ((l->newkey == 2) && (!l->lastrealrx)) {
if ((l->newkey == KEYUP_NOT_ALLOWED) && (!l->lastrealrx)) {
rxkey_helper(myrpt, l);
}
if (((l->phonemode) && (l->phonevox)) || (!strcasecmp(ast_channel_tech(l->chan)->type, "echolink"))
Expand Down Expand Up @@ -4485,7 +4487,7 @@ static inline int process_link_channels(struct rpt *myrpt, struct ast_channel *w
l->reconnects++;
}
/* if RX key */
if ((f->subclass.integer == AST_CONTROL_RADIO_KEY) && (l->newkey < 2)) {
if ((f->subclass.integer == AST_CONTROL_RADIO_KEY) && (l->newkey != KEYUP_NOT_ALLOWED)) {
rxkey_helper(myrpt, l);
}
/* if RX un-key */
Expand Down Expand Up @@ -4522,11 +4524,11 @@ static inline int process_link_channels(struct rpt *myrpt, struct ast_channel *w
fac_frame(f, fac);
}
/* foop */
if (l->chan && (l->lastrx || (!altlink(myrpt, l))) && ((l->newkey < 2) || l->lasttx || strcasecmp(ast_channel_tech(l->chan)->type, "IAX2"))) {
if (l->chan && (l->lastrx || (!altlink(myrpt, l))) && ((l->newkey != KEYUP_NOT_ALLOWED) || l->lasttx || strcasecmp(ast_channel_tech(l->chan)->type, "IAX2"))) {
/* Reverse-engineering comments from NA debugging issue #46:
* We may be receiving frames from channel drivers but we discard them and don't pass them on if newkey hasn't been set to 2 yet.
* Of course if handle_link_data is never called to set newkey to 2 and stop newkeytimer, then at some point, we'll
* set newkey = 0 forcibly (see comments in that part of the code for more info), which will cause us to start passing on the voice frames here.
* We may be receiving frames from channel drivers but we discard them and don't pass them on if newkey hasn't been set to != KEYUP_NOT_ALLOWED yet.
* Of course if handle_link_data is never called to set newkey to KEYUP_NOT_ALLOWED and stop newkeytimer, then at some point, we'll
* set newkey = KEYUP_ALLOWED forcibly (see comments in that part of the code for more info), which will cause us to start passing on the voice frames here.
* If this happens, then we're passing voice frames so we're keyed up and transmitting, essentially.
*/
ast_write(l->chan, f);
Expand Down Expand Up @@ -4583,7 +4585,7 @@ static inline int monchannel_read(struct rpt *myrpt)
/* go thru all the links */
while (l != &myrpt->links) {
/* foop */
if (l->chan && altlink(myrpt, l) && (!l->lastrx) && ((l->newkey < 2) || l->lasttx || strcasecmp(ast_channel_tech(l->chan)->type, "IAX2"))) {
if (l->chan && altlink(myrpt, l) && (!l->lastrx) && ((l->newkey != KEYUP_NOT_ALLOWED) || l->lasttx || strcasecmp(ast_channel_tech(l->chan)->type, "IAX2"))) {
if (l->chan && (!strcasecmp(ast_channel_tech(l->chan)->type, "irlp"))) {
ast_write(l->chan, fs);
} else {
Expand Down Expand Up @@ -5857,7 +5859,7 @@ static inline int exec_chan_read(struct rpt *myrpt, struct ast_channel *chan, ch
if (f->frametype == AST_FRAME_VOICE) {
struct ast_frame *f1;
int ismuted;
if (myrpt->newkey == 2) {
if (myrpt->newkey == KEYUP_NOT_ALLOWED) {
myrpt->rxlingertimer = ((myrpt->iaxkey) ? RX_LINGER_TIME_IAXKEY : RX_LINGER_TIME);
if (!*keyed) {
*keyed = 1;
Expand Down Expand Up @@ -5959,7 +5961,7 @@ static inline int exec_chan_read(struct rpt *myrpt, struct ast_channel *chan, ch
return -1;
}
/* if RX key */
if ((f->subclass.integer == AST_CONTROL_RADIO_KEY) && (myrpt->newkey < 2)) {
if ((f->subclass.integer == AST_CONTROL_RADIO_KEY) && (myrpt->newkey != KEYUP_NOT_ALLOWED)) {
ast_debug(7, "@@@@ rx key\n");
*keyed = 1;
myrpt->rerxtimer = 0;
Expand Down Expand Up @@ -6029,7 +6031,7 @@ static inline int exec_pchannel_read(struct rpt *myrpt, struct ast_channel *chan
return -1;
}
if (f->frametype == AST_FRAME_VOICE) {
if ((myrpt->newkey < 2) || myrpt->remoterx || strcasecmp(ast_channel_tech(chan)->type, "IAX2")) {
if ((myrpt->newkey != KEYUP_NOT_ALLOWED) || myrpt->remoterx || strcasecmp(ast_channel_tech(chan)->type, "IAX2")) {
ast_write(chan, f);
}
}
Expand Down Expand Up @@ -6686,10 +6688,10 @@ static int rpt_exec(struct ast_channel *chan, const char *data)
l->gott = 0;
l->rxlingertimer = ((l->iaxkey) ? RX_LINGER_TIME_IAXKEY : RX_LINGER_TIME);
l->newkeytimer = NEWKEYTIME;
l->newkey = 0;
l->newkey = KEYUP_ALLOWED;
l->iaxkey = 0;
if ((!phone_mode) && (l->name[0] != '0') && strcasecmp(ast_channel_tech(chan)->type, "echolink") && strcasecmp(ast_channel_tech(chan)->type, "tlb")) {
l->newkey = 2;
l->newkey = KEYUP_NOT_ALLOWED;
}
ast_debug(7, "newkey: %d\n", l->newkey);
if (l->name[0] > '9') {
Expand Down Expand Up @@ -7005,7 +7007,7 @@ static int rpt_exec(struct ast_channel *chan, const char *data)
myrpt->reload = 0;
myrpt->tele.next = &myrpt->tele;
myrpt->tele.prev = &myrpt->tele;
myrpt->newkey = 0;
myrpt->newkey = KEYUP_ALLOWED;
myrpt->iaxkey = 0;
myrpt->lastitx = !myrpt->lastitx;
myrpt->tunerequest = 0;
Expand Down Expand Up @@ -7069,7 +7071,7 @@ static int rpt_exec(struct ast_channel *chan, const char *data)
}
/* if is a webtransceiver */
if (myrpt->remote_webtransceiver)
myrpt->newkey = 2;
myrpt->newkey = KEYUP_NOT_ALLOWED;
myrpt->loginuser[0] = 0;
myrpt->loginlevel[0] = 0;
myrpt->authtelltimer = 0;
Expand Down Expand Up @@ -7200,7 +7202,7 @@ static int rpt_exec(struct ast_channel *chan, const char *data)
}
keyed = myrx;
update_rxlingertimer(myrpt, elap);
if ((myrpt->newkey == 2) && keyed && (!myrpt->rxlingertimer)) {
if ((myrpt->newkey == KEYUP_NOT_ALLOWED) && keyed && (!myrpt->rxlingertimer)) {
myrpt->rerxtimer = 0;
keyed = 0;
}
Expand Down Expand Up @@ -7239,7 +7241,7 @@ static int rpt_exec(struct ast_channel *chan, const char *data)

if (rem_rx && !myrpt->remoterx) {
myrpt->remoterx = 1;
if (myrpt->newkey < 2)
if (myrpt->newkey != KEYUP_NOT_ALLOWED)
ast_indicate(chan, AST_CONTROL_RADIO_KEY);
}
if (!rem_rx && myrpt->remoterx) {
Expand All @@ -7256,7 +7258,7 @@ static int rpt_exec(struct ast_channel *chan, const char *data)
break; /* if not logged in, hang up after a time */
}
}
if (myrpt->newkey == 1) {
if (myrpt->newkey == KEYUP_ALLOWED_REDUNDANT) {
if ((myrpt->retxtimer += elap) >= REDUNDANT_TX_TIME) {
myrpt->retxtimer = 0;
if ((myrpt->remoterx) && (!myrpt->remotetx))
Expand Down
11 changes: 9 additions & 2 deletions apps/app_rpt/app_rpt.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,13 @@ struct rpt_chan_stat {
#define NEWKEY1STR "!NEWKEY1!"
#define IAXKEYSTR "!IAXKEY!"

/*! \brief Repeater link connection newkey handshake state */
enum rpt_newkey {
KEYUP_ALLOWED, /*!< Radio keyup is allowed from link */
KEYUP_ALLOWED_REDUNDANT, /*!< "!NEWKEY!" - radio keyups allowed from link */
KEYUP_NOT_ALLOWED /*!< On initial connection before reciept of "!NEWKEY1!" message - no radio keyups are allowed */
}

struct vox {
float speech_energy;
float noise_energy;
Expand Down Expand Up @@ -418,7 +425,7 @@ struct rpt_link {
char wasvox;
int voxtotimer;
char voxtostate;
char newkey;
enum rpt_newkey newkey;
char iaxkey;
int linkmode;
int newkeytimer;
Expand Down Expand Up @@ -840,7 +847,7 @@ struct rpt {
int linkposttimer;
int keyposttimer;
int lastkeytimer;
char newkey;
enum rpt_newkey newkey;
char iaxkey;
char inpadtest;
long rxlingertimer;
Expand Down
2 changes: 1 addition & 1 deletion apps/app_rpt/rpt_channel.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ void send_newkey(struct ast_channel *chan)
return;
}

void send_old_newkey(struct ast_channel *chan)
void send_newkey_redundant(struct ast_channel *chan)
{
ast_channel_lock(chan);
if (ast_sendtext(chan, NEWKEYSTR)) {
Expand Down
2 changes: 1 addition & 1 deletion apps/app_rpt/rpt_channel.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,4 @@ int send_link_pl(struct rpt *myrpt, char *txt);
/*! \brief send newkey request */
void send_newkey(struct ast_channel *chan);

void send_old_newkey(struct ast_channel *chan);
void send_newkey_redundant(struct ast_channel *chan);
4 changes: 2 additions & 2 deletions apps/app_rpt/rpt_link.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -678,10 +678,10 @@ int connect_link(struct rpt *myrpt, char *node, int mode, int perma)
l->hasconnected = l->perma = perma;
l->newkeytimer = NEWKEYTIME;
l->iaxkey = 0;
l->newkey = 2;
l->newkey = KEYUP_NOT_ALLOWED;
l->voterlink = voterlink;
if (strncasecmp(s1, "echolink/", 9) == 0) {
l->newkey = 0;
l->newkey = KEYUP_ALLOWED;
}
if (!strncasecmp(s1, "iax2/", 5) || !strncasecmp(s1, "echolink/", 9) || !strncasecmp(s1, "tlb/", 4)
#ifdef ALLOW_LOCAL_CHANNELS
Expand Down

0 comments on commit 7660f5c

Please sign in to comment.