New important fix. And: Many new features. bug fixes and improvements

Last commit from today lead to a stack smash (buffer overflow) in is_call_valid().
Found the bug -> fixed with this commit.

is_call_blacklisted() and the new handle_aprs_messsage_addressed_to_us() function
may be called concurrently in the main program thread and in the taskWebServer-thread.
I'm not really sure if buffer operation on the local variable is thread safe; that's
why I added sema locking for both. Needed? Not? I'd be glad to remove it ;)

handle_aprs_messsage_addressed_to_us() returns a String. I'm also not sure what
happens on concurrency -> I now return a copy of the String: String(answer_message).

The changelog of the last commit of today:
==========================================

New features
------------

- personal text messages addressed to the tracker
  - are now recognized and stay local -> will not be digipeated or sent to aprsis
     see handle_aprs_messsage_addressed_to_us(), used in the main program (for
     rf-received packets) and in taskWebServer for packets received from aprs-is
     It also displays the message
        +      writedisplaytext(" ((MSG))","",String(msg_from) + String(": ") + String(header_normal_or_third_party_end + 11+1),"","","");
     for 30s:
        +      display_dont_update_until = millis() + 30000L;
     We display the msg in favour of the RX packet.
  - we send an ACK when a message-ID is present
  - This is also useful for the winlink feature described below

- Winlink feature
  - if enabled, your device will add the word @winlink to the position beacon text
    every hour (Refresh interval at winlink is two hours).
    You can choose if it is sent always, or only if a bluetooth device (i.e. aprsdroid)
    is connected.
    Winlink documented that you could send a "status packet" instead. This would be
    really nice and I implemented it - but they ignore it. -> I've commented
    it our (for not loosing it), and switched to the position comment approach.
  - If we get a notification, a "W" is in the display as indicator that new messages
    are waiting for you.
    The W is displayed for 24h. Because winlink only informs every 24 hours, and the
    tracker does not know if you read the message at the winlink system

- Powerdown:
  - If you disconnect external power (USB) from the TTGO device, it can shutdown if
    you configured the shutdown delay. This is useful when operating in the car and
    you park. Someone configured a shutdown delay of 1h. -> Shutdown info is displayed
    a few seconds (you can interrupt the shutdown by pressing the middle button), and
    info is displayed again a few seconds right before the actual shutdown (also with
    the option do interrupt the shutdown by pressing the middle button).
  - On shutdown, a message QRT status message could be sent automatically, if configured

- LoRa APRS digipeating: you can configure now an alias reacting as digipeater
  (kenwood names it "UITRACE". We react only on this call if it's in the first
  position of the path and has no digipeated flag.

- Serial console tnc cli:
  - Show reboot and halt info also at the display if you issue the reboot or shutdown command
    And send a status packet if configured (see new status packet feature described above)

Bug fixes
---------
- Fix for AXP2101 on T-Beam V1.2:
  In setup() (setup_phase2_soft_reconfiguration()), we needed to add
    +        axp.setButtonBatteryChargeVoltage(3300);                             // enable charge of the gps battery
    +        axp.enableButtonBatteryCharge();
  else the button battery (which is a capacitor) never became loaded. This lead
  to a time of 10 min to get a GPS fix after powering on (because it needed to wait
  for the almanach)
- Fix and improve of smartbeaconing computation. Sometimes the timer went too fast down,
  sometimes it went up to max again -> we mostly relied on the course-change event.
  You will really notice the improvement.
- Wrong info if lora TX is enabled and current state of dont_send_own_position_packets
  is true. Display showed "LoRa-TX; dis", but only the active sending of the
  beacon is stopped:
    -  if (dont_send_own_position_packets || !(lora_tx_enabled || aprsis_enabled)) {
    +  if (dont_send_own_position_packets) {
    +    OledLine2 = wifi_info + " Own Bcn: dis";
    +  } else if (!(lora_tx_enabled || aprsis_enabled)) {
         OledLine2 = wifi_info + " LoRa-TX: dis";
- Beautifying the lines in the display output (indents for different speed units,
  missing blank if no "p" or "P", etc.
  I had developed that with the device with-display-and-no-GPS and saw the indent
  errors at the moment I used my new-device-with-display-and-GPS
- During debugging, I was not able to send a packet with digipeated flag via serial
  console converse mode: typo (q instead of p check):
    -          if (! (*p == '-' || *p == '>' || *p == ',' || *q == '*' || (*p >= 'A' && *p <= 'Z') || (*p >= '0' && *p <= '9')))
    +          if (! (*p == '-' || *p == '>' || *p == ',' || *p == '*' || (*p >= 'A' && *p <= 'Z') || (*p >= '0' && *p <= '9')))
  ;))
- Moved update_speed_from_gps() higher, before gps.speed() calls, because update_speed_from_gps()
  implicitly changes the gps.speed.isUpdated() value during his calls. This lead to unexpected fewer conditions of speed updates
- Improved gps location / speed / ... handling:
  Work on bestHdop section. You know, gps jumps at low speeds, and it's hard to find a good value
  between needed updates and wrong-positives. I left old values commented-out in the code. In my test, it now behaves much better.
  If situation is "p" (had a fix, but lost it), not only set aprsPresetShown "p" but also call displayInvalidGPS();
  If situation is " ": we had no gps fix, but we now have a fix:
           // No GPS signal for a long time? Enforce tx:
    +      if ((millis() - lastPositionTX) > ((sb_max_interval - sb_min_interval) / 2))
    +        nextTX = sb_min_interval;
  Sounds good?
- removed time_to_refresh = millis() + showRXTime from handle_usb_serial_input(), KISSTX, TX-Xdigi, ..
  because this value is only intended for the time of displaying ((RX)-packets and must not be
  overloaded by other stuff. See also new variable display_dont_update_until.
- taskWebServer send_queue_to_aprsis():
  While \r is a valid char in an aprs message text (for separating lines), it must not be
    sent to aprs-is, because they interpret \r, \n, \r\n as EOL. That means, the rest of the
    line will be interpreted and may lead to a disconnect due to garbled input.
    -> +  data.replace("\r", " "); data.replace("\n", " "); data.replace("\0", " ");
- is_call_blacklisted(): moved strcat after length check:
    -      // our ssid filter construct: -0 means search for call with ssid 0 zero.
    -      if (!(r = strchr(buf, '-')))
    -        strcat(buf, "-0");
           // after modifications above, is len(buf) still < 10 (space for ',' and \0)?
           if (strlen(buf) > 10)
             return 0;
    +      // our ssid filter construct: -0 means search for call with ssid 0 zero.
    +      if (!(r = strchr(buf, '-')))
    +        strcat(buf, "-0");
- Fixed: average_speed array: last field was always zero (%4 instead of %5)
    -  average_speed[point_avg_speed] = (((curr_hdop < 1.5 && curr_sats >= 3 && curr_kmph <= 16.0) || (curr_hdop < 4.0 && curr_sats >= 3 && curr_kmph > 16.0)) ? curr_kmph : average_speed[(point_avg_speed-1) % 4]);   // calculate smart beaconing. Even a not-updated old speed is ok here.
    +  average_speed[point_avg_speed] = ((curr_kmph < 1.8 || (curr_hdop < 1.5 && curr_sats >= 3 && curr_kmph <= 16.0) || (curr_hdop < 4.0 && curr_sats >= 3 && curr_kmph > 16.0)) ? curr_kmph : average_speed[(point_avg_speed-1) % 5]);   // calculate smart beaconing. Even a not-updated old speed is ok here.

average_speed_final = (average_speed[0]+average_speed[1]+average_speed[2]+average_speed[3]+average_speed[4])/5;

Other changes
-------------

- Instead of
    if (enable_bluetooth && SerialBT.hasClient()){
  we now have
    if (serial_bt_client_is_connected){
  in TTGO_T-Beam_LoRa_APRS.ino, taskWebServer.cpp and taskTNC.cpp
  and the current state is checked in the main loop.
  This way we also learn if a new serial_bt_client is connected
- Oled: new display_dont_update_until prevents display changes -> important messages
  are not interrupted
- Better space handling for beacon text additions like "@winlink" "Batt=", "P=", " !W..!"
- nextTX and next_fixed_beacon adjustments moved now in sendpacket(), because
  they need to be adjusted (and only then) after calling this function:
    +  lastPositionTX = millis();
    +  // reset timer for automatic fixed beacon after manual beacon
    +  next_fixed_beacon = millis() + fix_beacon_interval;
    +  nextTX = sb_max_interval;
- enableOled_now() enforces Oled to be on (without waiting for the next loop in main loop).
  display_dont_update_until is set to the needed values
- variable name lastTX changed to lastPositionTX because this variable is only needed
  in the context of the last position transmission. -> the name lastTX was misleading.
- is_call_blacklisted():
  whitelist call "WLNK", because it has to be valid for winlink communication
- at various places in the code:
  moved display routines like writedisplaytext("((KISSTX))","",inputBuf,"","",""); to the
  place before loraSend -> more time for the user to read the message
- Middle button code:
    - fillDisplayLines3to5(0) added to MAN TX / FIX TX part
    - handle the power-off-timer condition (-> timeslots for being able to abort the poweroff)
- variables t_last_smart_beacon_sent and lastPositionTX had the same meaning.
  -> Abandoned t_last_smart_beacon_sent.
- New tmp_t_since_last_sb_tx in the loop: avoid unnecessary recomputations of
  millis() - lastPositionTX at various places afterwards in the loop.
- taskWebserver:
  - On shutdown or reboot, send statusPacket if configured.
    And set a delay for do_send_status_message_about_shutdown_to_aprsis()
  - code beautify in handle_SaveAPRSCfg() and read_from_aprsis() (here: changed
    log message read_from_aprs to the more correct read_from_aprsis).
  - aprs-message ack-handling (described above)
  - renamed send_to_aprsis() to send_queue_to_aprsis(), because we have in  TTGO_T-Beam_LoRa_APRS.ino
    the same function (which only fills the queue, which is handled by send_queue_to_aprsis() in
    taskWebServer.cpp).
  - More comprehensible:
      -                        <label for="aprs_llfgps">Use lat/lon of current GPS position</label>
      +                        <label for="aprs_llfgps">Instead, store current GPS position</label>

Signed-off-by: Thomas Osterried <dl9sau@darc.de>

src/TTGO_T-Beam_LoRa_APRS.ino

@@ -523,8 +523,12 @@ Adafruit_SSD1306 display(128, 64, &Wire, OLED_RESET);
 
 #ifdef IF_SEMAS_WOULD_WORK
   xSemaphoreHandle sema_lora_chip;
+  xSemaphoreHandle sema_handle_aprs_message_addressed_to_us;
+  xSemaphoreHandle sema_is_call_blacklisted;
 #else
   volatile boolean sema_lora_chip = false;
+  volatile boolean sema_handle_aprs_message_addressed_to_us = false;
+  volatile boolean sema_is_call_blacklisted = false;
 #endif
 
 
@@ -3891,8 +3895,12 @@ void setup()
   // Avoid concurrent access of processes to lora our chip
 #ifdef IF_SEMAS_WOULD_WORK
   sema_lora_chip = xSemaphoreCreateBinary();
+  sema_handle_aprs_message_addressed_to_us = xSemaphoreCreateBinary();
+  sema_is_call_blacklisted = xSemaphoreCreateBinary();
 #else
   sema_lora_chip = false;
+  sema_handle_aprs_message_addressed_to_us = false;
+  sema_is_call_blacklisted = false;
 #endif
 
 
@@ -4039,10 +4047,30 @@ int packet_is_valid (const char *frame_start) {
 
 
 int is_call_blacklisted(const char *frame_start) {
+
+  esp_task_wdt_reset();
+  #ifdef IF_SEMAS_WOULD_WORK
+    while (xSemaphoreTake(sema_is_call_blacklisted, 100) != pdTRUE)
+      esp_task_wdt_reset();
+  #else
+    for (int n = 0; sema_is_call_blacklisted; n++) {
+      delay(10);
+      if (!(n % 100))
+        esp_task_wdt_reset();
+      }
+    sema_is_call_blacklisted = true;
+  #endif
+
   // src-call_validation
   const char *p_call = frame_start;
+  char *header_end;
+  char *p;
+  int ret = 1;
+  boolean ssid_present = false;
+  char buf[13]; // room for ",DL1AAA-15*," + \0
   int i = 0;
-  if (!p_call || !*p_call) return 1;
+
+  if (!p_call || !*p_call) goto end;
   // Special case: message from ham radio winlink system:
   if (!strncmp(p_call, "WLNK", 4)) {
     ;
@@ -4054,19 +4082,18 @@ int is_call_blacklisted(const char *frame_start) {
       p_call--; // warning, beyond start of pointer
     }
     for (; i <= 7; i++) {
-      if (i == 7) return 1;
+      if (i == 7) goto end;
       else if (!p_call[i] || p_call[i] == '>' || p_call[i] == '-') {
-        if (i < 4) return 1;
+        if (i < 4) goto end;
         break;
-      } else if (i < 2 && !isalnum(p_call[i])) return 1;
-      else if (i == 2 && !isdigit(p_call[i])) return 1;
-      else if (i > 2 && !isalpha(p_call[i])) return 1;
+      } else if (i < 2 && !isalnum(p_call[i])) goto end;
+      else if (i == 2 && !isdigit(p_call[i])) goto end;
+      else if (i > 2 && !isalpha(p_call[i])) goto end;
     }
   }
 
-  boolean ssid_present = false;
-  char buf[12]; // room for ",DL1AAA-15," + \0
-  char *p = buf;
+
+  p = buf;
   *p++ = ',';
   for (i = 0; i < 9; i++) {
     if (!frame_start[i] || /* frame_start[i] == '>' || */ ! (frame_start[i] == '-' || isalnum(frame_start[i]))) {
@@ -4093,20 +4120,26 @@ int is_call_blacklisted(const char *frame_start) {
 
 
   // blacklist empty? we may leave here
-  if (!*blacklist_calls || !strcmp(blacklist_calls, ",,"))
-    return 0;
+  if (!*blacklist_calls || !strcmp(blacklist_calls, ",,")) {
+    ret = 0;
+    goto end;
+  }
 
   // exact match?
-  if (strstr(blacklist_calls, buf))
-    return 2;
+  if (strstr(blacklist_calls, buf)) {
+    ret = 2;
+    goto end;
+  }
   // filter call completely?
   if ((p = strchr(buf, '-'))) {
     *p++ = ','; *p++ = 0;
-    if (strstr(blacklist_calls, buf))
-      return 3;
+    if (strstr(blacklist_calls, buf)) {
+      ret = 3;
+      goto end;
+    }
   }
 
-  char *header_end = strchr(frame_start, ':');
+  header_end = strchr(frame_start, ':');
   // check for blacklisted digi in path
   if (header_end && (p = strchr(frame_start, ','))) {
     for (;;) {
@@ -4114,7 +4147,7 @@ int is_call_blacklisted(const char *frame_start) {
       if (!q || q > header_end)
         q = header_end;
       // copy ",DL1AAA,.." to buf as ",DL1AAA"
-      // but before: length check. len ",DL1AAA-15*," is 12; sizeof(buf) is 12 (due to \0); we copy until trailing ','.
+      // but before: length check. len ",DL1AAA-15*," is 12; sizeof(buf) is 13 (due to \0); we copy until trailing ','.
       if ((q-p) > sizeof(buf)-1)
         break;
       strncpy(buf, p, q-p);
@@ -4122,29 +4155,45 @@ int is_call_blacklisted(const char *frame_start) {
       char *r = strchr(buf, '*');
       if (r)
         *r = 0;
-      // after modifications above, is len(buf) still < 10 (space for ',' and \0)?
-      if (strlen(buf) > 10)
-        return 0;
+      // after modifications above, is len(buf) still <= 10 (room for ',' + 9 (call-ssid)  == 10. and later  ',' and \0)?
+      r = strchr(buf, '-');
+      if (strlen(buf) > (r ? 10 : 7)) {
+        // can't check non-conformal stuff. Unfortunately, aprs-is tier node names in the digi path like T2CSNGRAD (length of 9)
+        // are non-conformal. That's why we return 0 here. Length for our exact-match-test below would be ",T2CSNGRAD-0,"+\0 == 14
+        break;
+      }
       // our ssid filter construct: -0 means search for call with ssid 0 zero.
-      if (!(r = strchr(buf, '-')))
+      if (!r) {
         strcat(buf, "-0");
+      }
       strcat(buf, ",");
       // exact match?
-      if (strstr(blacklist_calls, buf))
-        return 4;
+      if (strstr(blacklist_calls, buf)) {
+        ret = 4;
+        goto end;
+      }
       // filter call completely?
       if ((r = strchr(buf, '-'))) {
         *r++ = ','; *r++ = 0;
-        if (strstr(blacklist_calls, buf))
-          return 5;
+        if (strstr(blacklist_calls, buf)) {
+          ret = 5;
+          goto end;
+        }
       }
       if (q == header_end)
         break;
       p = q;
     }
   }
+  ret = 0;
 
-  return 0;
+end:
+  #ifdef IF_SEMAS_WOULD_WORK
+    xSemaphoreGive(sema_is_call_blacklisted);
+  #else
+    sema_is_call_blacklisted = false;
+  #endif
+  return ret;
 }
 
 
@@ -5194,32 +5243,49 @@ void handle_usb_serial_input(void) {
 
 
 String handle_aprs_messsage_addressed_to_us(const char *received_frame) {
+  char *header_end;
+  const char *header_normal_or_third_party_start;
+  char *header_normal_or_third_party_end;
+  char *q;
+
+  #ifdef IF_SEMAS_WOULD_WORK
+    while (xSemaphoreTake(sema_handle_aprs_message_addressed_to_us, 100) != pdTRUE)
+      esp_task_wdt_reset();
+  #else
+    for (int n = 0; sema_handle_aprs_message_addressed_to_us; n++) {
+      delay(10);
+      if (!(n % 100))
+        esp_task_wdt_reset();
+    }
+    sema_handle_aprs_message_addressed_to_us = true;
+  #endif
+
   String answer_message = "";
   if (!received_frame)
-    return answer_message;
-  char *header_end = strchr(received_frame, ':');
+    goto end;
+  header_end = strchr(received_frame, ':');
   if (!header_end)
-    return answer_message;
-  const char *header_normal_or_third_party_start = received_frame;
-  char *header_normal_or_third_party_end = header_end;
-  char *q;
+    goto end;
+  header_normal_or_third_party_start = received_frame;
+  header_normal_or_third_party_end = header_end;
 
   // handle messages addressed to us. May be called after lora RF receiption, or when parsing an aprsis message
   if (header_end[1] == '}') {
     header_normal_or_third_party_start = header_end+2;
     header_normal_or_third_party_end = strchr(header_normal_or_third_party_start, ':');
     if (!header_normal_or_third_party_end)
-      return answer_message;
+      goto end;
   }
 
-  if (is_call_blacklisted(header_normal_or_third_party_start))
-    return answer_message;
-
   if (strlen(header_normal_or_third_party_end+1) > 11 &&
       header_normal_or_third_party_end[1] == ':' &&
       header_normal_or_third_party_end[11] == ':') {
     // this is an aprs message. Is this message for us? Then we will not digipeat and we will not send it to aprsis
     // Example: WLNK-1>APWLK,TCPIP*,qAC,T2MCI::DL9SAU-12:You have 1 Winlink mail messages pending{2077
+
+    if (is_call_blacklisted(header_normal_or_third_party_start))
+      goto end;
+
     char msg_to[12];
     sprintf(msg_to, ":%-9.9s:", Tcall.c_str());
     if (!strncmp(header_normal_or_third_party_end+1, msg_to, 11)) {
@@ -5261,7 +5327,14 @@ String handle_aprs_messsage_addressed_to_us(const char *received_frame) {
     }
   }
 
-  return answer_message;
+end:
+  #ifdef IF_SEMAS_WOULD_WORK
+    xSemaphoreGive(sema_handle_aprs_message_addressed_to_us);
+  #else
+    sema_handle_aprs_message_addressed_to_us = false;
+  #endif
+
+  return String(answer_message);
 }