Add non-blocking BBMD registration API; drop the self-poll fallback from #66

src/bacnet/basic/bbmd/h_bbmd.c

@@ -1160,10 +1160,14 @@ int bvlc_handler(BACNET_IP_ADDRESS *addr,
  *         0 if no registration request is sent, or
  *         -1 if registration fails.
  */
-int bvlc_register_with_bbmd(BACNET_IP_ADDRESS *bbmd_addr, uint16_t ttl_seconds)
+/* Shared by bvlc_register_with_bbmd() and bvlc_register_with_bbmd_async():
+ * marks bbmd_addr as our BBMD (so outbound broadcasts route through it via
+ * bvlc_send_pdu()'s Remote_BBMD.port check, immediately -- see that
+ * function -- regardless of whether this registration is ever ACKed),
+ * resets the registration status, and sends the request. Returns whatever
+ * bip_send_mpdu() returns. */
+static int send_register_foreign_device_request(BACNET_IP_ADDRESS *bbmd_addr, uint16_t ttl_seconds)
 {
-    /* Store the BBMD address and port so that we won't broadcast locally. */
-    /* We are a foreign device! */
     bvlc_address_copy(&Remote_BBMD, bbmd_addr);
     BVLC_Buffer_Len = bvlc_encode_register_foreign_device(
         &BVLC_Buffer[0], sizeof(BVLC_Buffer), ttl_seconds);
@@ -1173,71 +1177,96 @@ int bvlc_register_with_bbmd(BACNET_IP_ADDRESS *bbmd_addr, uint16_t ttl_seconds)
     bbmd_reg = BBMD_REG_UNSET;
     pthread_mutex_unlock (&mutex);
 
-    int retval = bip_send_mpdu(bbmd_addr, &BVLC_Buffer[0], BVLC_Buffer_Len);
+    return bip_send_mpdu(bbmd_addr, &BVLC_Buffer[0], BVLC_Buffer_Len);
+}
+
+/**
+ * Sends a Register-Foreign-Device request and returns immediately --
+ * does not wait for the BVLC-Result ACK/NAK. Intended for a caller (such
+ * as bacnet-sim) that is single-threaded and hasn't started its own
+ * receive loop yet at the point it registers: unlike
+ * bvlc_register_with_bbmd(), this never itself reads the socket, so it
+ * introduces no risk of stealing a datagram from whatever receive loop
+ * the caller starts afterward. Poll bvlc_bbmd_registration_status()
+ * once that loop is running to find out whether it actually succeeded.
+ *
+ * @param bbmd_addr - BBMD address to register with
+ * @param ttl_seconds - lease time to request
+ * @return Upon successful completion (of the *send*, not the
+ *  registration itself -- see bvlc_bbmd_registration_status()), returns
+ *  the number of bytes sent. Otherwise, -1 shall be returned.
+ */
+int bvlc_register_with_bbmd_async(BACNET_IP_ADDRESS *bbmd_addr, uint16_t ttl_seconds)
+{
+    return send_register_foreign_device_request(bbmd_addr, ttl_seconds);
+}
+
+/**
+ * Reports the outcome of the most recent bvlc_register_with_bbmd_async()
+ * call. Thread-safe; cheap enough to poll from a main loop.
+ *
+ * @return BVLC_BBMD_REGISTRATION_PENDING if no BVLC-Result has been
+ *  processed yet (call again later, once the caller's own receive loop
+ *  has had a chance to run), BVLC_BBMD_REGISTRATION_FAILED or
+ *  BVLC_BBMD_REGISTRATION_SUCCEEDED once one has.
+ */
+BVLC_BBMD_REGISTRATION_STATUS bvlc_bbmd_registration_status(void)
+{
+    pthread_mutex_lock (&mutex);
+    bbmd_reg_t status = bbmd_reg;
+    pthread_mutex_unlock (&mutex);
+
+    switch (status)
+    {
+        case BBMD_REG_SUCCESS:
+            return BVLC_BBMD_REGISTRATION_SUCCEEDED;
+        case BBMD_REG_FAIL:
+            return BVLC_BBMD_REGISTRATION_FAILED;
+        case BBMD_REG_UNSET:
+        default:
+            return BVLC_BBMD_REGISTRATION_PENDING;
+    }
+}
+
+/**
+ * Sends a Register-Foreign-Device request and blocks for up to 30
+ * seconds waiting for the BVLC-Result ACK/NAK.
+ *
+ * This wait relies on something else already reading this socket in a
+ * loop -- e.g. a caller's own pre-existing receive thread, such as
+ * device-bacnet-c-private's receive_data(), which starts before it ever
+ * registers -- to deliver the reply via bvlc_handler()'s normal signal
+ * path. A single-threaded caller with no such loop running yet at the
+ * point it registers (e.g. bacnet-sim, calling this from main() before
+ * starting its own receive loop) has nothing to signal it and will
+ * simply time out here even on a genuinely successful registration; such
+ * a caller should use bvlc_register_with_bbmd_async() +
+ * bvlc_bbmd_registration_status() instead, which never blocks and so
+ * never depends on anything else servicing the socket concurrently.
+ */
+int bvlc_register_with_bbmd(BACNET_IP_ADDRESS *bbmd_addr, uint16_t ttl_seconds)
+{
+    int retval = send_register_foreign_device_request(bbmd_addr, ttl_seconds);
     if (retval == -1)
     {
         return retval;
     }
 
     bool received_response = false;
-    time_t start = time(NULL);
-    time_t deadline = start + 30;
-
-    /* Phase 1: give whatever's already reading this socket -- e.g. a
-     * caller's own pre-existing receive thread, such as
-     * device-bacnet-c-private's receive_data(), which starts before it
-     * ever registers -- the normal signal path's chance to deliver the
-     * reply first. As long as something else is already pumping
-     * bip_receive() in a loop, that path works correctly and never
-     * competes with it for datagrams on the socket, exactly as it always
-     * has. A short, bounded grace period is enough: on a real network an
-     * already-running receive thread signals within well under a second
-     * of the BBMD's reply arriving. */
-    time_t grace_deadline = start + 2;
-    while (!received_response && time(NULL) < grace_deadline)
+    time_t deadline = time(NULL) + 30;
+    while (!received_response && time(NULL) < deadline)
     {
         struct timeval now;
         struct timespec timeout;
         gettimeofday (&now, NULL);
-        timeout.tv_sec = now.tv_sec;
-        timeout.tv_nsec = (now.tv_usec + 250000) * 1000;
-        if (timeout.tv_nsec >= 1000000000)
-        {
-            timeout.tv_sec += 1;
-            timeout.tv_nsec -= 1000000000;
-        }
+        timeout.tv_sec = now.tv_sec + 1;
+        timeout.tv_nsec = now.tv_usec * 1000;
         pthread_mutex_lock (&mutex);
         pthread_cond_timedwait (&cond, &mutex, &timeout);
         received_response = bbmd_reg != BBMD_REG_UNSET;
         pthread_mutex_unlock (&mutex);
     }
 
-    /* Phase 2: nobody signaled us within the grace period, which is the
-     * signature of a caller -- like bacnet-sim, which calls this
-     * synchronously from main() before starting any receive loop -- that
-     * has no other thread pumping the socket and so no way to ever be
-     * signaled by the normal path. Only now poll bip_receive() ourselves:
-     * it invokes bvlc_handler() internally on any packet received, which
-     * is what actually sets bbmd_reg. This does compete with any other
-     * reader of the socket for whichever datagram the OS hands to
-     * whichever caller asks first -- a real risk in general, since the
-     * loser silently drops a datagram that never reaches bvlc_handler()
-     * for anything but this fallback's own narrow use, but fine here
-     * because phase 1 already established nothing else is claiming
-     * datagrams from this socket. */
-    if (!received_response)
-    {
-        BACNET_ADDRESS bbmd_reply_src = { 0 };
-        uint8_t bbmd_reply_mtu[MAX_MPDU] = { 0 };
-        while (!received_response && time(NULL) < deadline)
-        {
-            bip_receive(&bbmd_reply_src, bbmd_reply_mtu, sizeof(bbmd_reply_mtu), 100);
-            pthread_mutex_lock (&mutex);
-            received_response = bbmd_reg != BBMD_REG_UNSET;
-            pthread_mutex_unlock (&mutex);
-        }
-    }
-
     /* Fail if the BBMD registration was not successful */
     if (bbmd_reg != BBMD_REG_SUCCESS)
     {

src/bacnet/basic/bbmd/h_bbmd.h

@@ -93,6 +93,32 @@ BACNET_STACK_EXPORT
 int bvlc_register_with_bbmd(
     BACNET_IP_ADDRESS *address, uint16_t time_to_live_seconds);
 
+/* Non-blocking counterpart to bvlc_register_with_bbmd(): sends the
+ * request and returns immediately, without waiting for or polling for
+ * the ACK/NAK itself. For a single-threaded caller that hasn't started
+ * its own receive loop yet at the point it registers (e.g. bacnet-sim),
+ * this avoids bvlc_register_with_bbmd()'s blocking wait entirely --
+ * that wait depends on something else already reading the socket to
+ * signal it, which such a caller has no way to provide. Start your
+ * receive loop right after this call returns, and check
+ * bvlc_bbmd_registration_status() once it's running.
+ */
+BACNET_STACK_EXPORT
+int bvlc_register_with_bbmd_async(
+    BACNET_IP_ADDRESS *address, uint16_t time_to_live_seconds);
+
+typedef enum
+{
+    BVLC_BBMD_REGISTRATION_PENDING,
+    BVLC_BBMD_REGISTRATION_FAILED,
+    BVLC_BBMD_REGISTRATION_SUCCEEDED
+} BVLC_BBMD_REGISTRATION_STATUS;
+
+/* Outcome of the most recent bvlc_register_with_bbmd_async() call. See
+ * its comment above. */
+BACNET_STACK_EXPORT
+BVLC_BBMD_REGISTRATION_STATUS bvlc_bbmd_registration_status(void);
+
 /* Local interface to manage BBMD.
  * The interface user needs to handle mutual exclusion if needed i.e.
  * BACnet packet is not being handled when the BBMD table is modified.

src/bacnet/datalink/dlenv.c

@@ -111,18 +111,16 @@ int dlenv_bbmd_result(void)
  *         0 if no registration request is sent, or
  *         -1 if registration fails.
  */
-int dlenv_register_as_foreign_device(void)
-{
-    int retval = 0;
 #if defined(BACDL_BIP)
-    bool bdt_entry_valid = false;
-    uint16_t bdt_entry_port = 0;
-    char *pEnv = NULL;
-    unsigned a[4] = { 0 };
-    char bbmd_env[32] = "";
-    unsigned entry_number = 0;
-    long long_value = 0;
-    int c;
+/* Reads BACNET_BBMD_ADDRESS/PORT/TIMETOLIVE into the BBMD_Address /
+ * BBMD_TTL_Seconds / BBMD_Address_Valid statics above. Shared by
+ * dlenv_register_as_foreign_device() and
+ * dlenv_register_as_foreign_device_async() so their env-var handling
+ * can't drift apart. */
+static void bbmd_env_config(void)
+{
+    char *pEnv;
+    long long_value;
 
     pEnv = getenv("BACNET_BBMD_PORT");
     if (pEnv) {
@@ -146,6 +144,63 @@ int dlenv_register_as_foreign_device(void)
     if (pEnv) {
         BBMD_Address_Valid = bip_get_addr_by_name(pEnv, &BBMD_Address);
     }
+}
+
+#endif
+
+/**
+ * Non-blocking counterpart to dlenv_register_as_foreign_device(): reads
+ * the same BACNET_BBMD_* environment variables, sends the
+ * Register-Foreign-Device request, and returns immediately -- it never
+ * waits for or logs the outcome. Intended for a single-threaded caller
+ * (e.g. bacnet-sim) that hasn't started its own receive loop yet at the
+ * point it registers: start that loop right after this call returns,
+ * then poll bvlc_bbmd_registration_status() (h_bbmd.h) once it's running
+ * to find out whether registration actually succeeded.
+ *
+ * Pair with dlenv_maintenance_timer_async() for periodic TTL renewal
+ * too -- a single-threaded caller using this function for startup
+ * should use that instead of dlenv_maintenance_timer(), which still
+ * calls the blocking dlenv_register_as_foreign_device() and would hit
+ * the same wait bvlc_register_with_bbmd() blocks on at every renewal.
+ *
+ * @return Positive number (of bytes sent) on a successful send, 0 if no
+ *  registration request is sent (BACNET_BBMD_ADDRESS unconfigured), or
+ *  -1 if the send itself failed.
+ */
+int dlenv_register_as_foreign_device_async(void)
+{
+    int retval = 0;
+#if defined(BACDL_BIP)
+    bbmd_env_config();
+    if (BBMD_Address_Valid) {
+        fprintf(stderr,
+            "Trying to register with BBMD at %u.%u.%u.%u:%u with TTL of %u seconds\n",
+            (unsigned)BBMD_Address.address[0],
+            (unsigned)BBMD_Address.address[1],
+            (unsigned)BBMD_Address.address[2],
+            (unsigned)BBMD_Address.address[3], (unsigned)BBMD_Address.port,
+            (unsigned)BBMD_TTL_Seconds);
+        retval = bvlc_register_with_bbmd_async(&BBMD_Address, BBMD_TTL_Seconds);
+        BBMD_Timer_Seconds = (uint16_t)BBMD_TTL_Seconds;
+    }
+#endif
+    return retval;
+}
+
+int dlenv_register_as_foreign_device(void)
+{
+    int retval = 0;
+#if defined(BACDL_BIP)
+    bool bdt_entry_valid = false;
+    uint16_t bdt_entry_port = 0;
+    char *pEnv = NULL;
+    unsigned a[4] = { 0 };
+    char bbmd_env[32] = "";
+    unsigned entry_number = 0;
+    int c;
+
+    bbmd_env_config();
     if (BBMD_Address_Valid) {
             fprintf(stderr,
                 "Trying to register with BBMD at %u.%u.%u.%u:%u with TTL of %u seconds\n",
@@ -363,6 +418,47 @@ void dlenv_maintenance_timer(uint16_t elapsed_seconds)
 #endif
 }
 
+/**
+ * Non-blocking counterpart to dlenv_maintenance_timer(): identical TTL
+ * countdown and renewal trigger, but fires
+ * dlenv_register_as_foreign_device_async() instead of the blocking
+ * dlenv_register_as_foreign_device() once the TTL expires. For a
+ * single-threaded caller (e.g. bacnet-sim) using the async registration
+ * API throughout, so periodic renewal doesn't hit the same wait
+ * bvlc_register_with_bbmd() blocks on -- one with no receive loop
+ * running concurrently has nothing to signal it. Call this instead of
+ * dlenv_maintenance_timer(), not in addition to it.
+ *
+ * @param elapsed_seconds Number of seconds that have elapsed since last
+ *  called.
+ * @return Same contract as dlenv_register_as_foreign_device_async():
+ *  positive (bytes sent) if a new request was just sent this call --
+ *  poll bvlc_bbmd_registration_status() for the outcome -- 0 if nothing
+ *  was due yet or BACNET_BBMD_ADDRESS is unconfigured, or -1 if renewal
+ *  was due but the send itself failed (nothing to poll for either way).
+ */
+int dlenv_maintenance_timer_async(uint16_t elapsed_seconds)
+{
+#if defined(BACDL_BIP)
+    if (BBMD_Timer_Seconds) {
+        if (BBMD_Timer_Seconds <= elapsed_seconds) {
+            BBMD_Timer_Seconds = 0;
+        } else {
+            BBMD_Timer_Seconds -= elapsed_seconds;
+        }
+        if (BBMD_Timer_Seconds == 0) {
+            int retval = dlenv_register_as_foreign_device_async();
+            BBMD_Timer_Seconds = (uint16_t)BBMD_TTL_Seconds;
+            return retval;
+        }
+    }
+    return 0;
+#else
+    (void)elapsed_seconds;
+    return 0;
+#endif
+}
+
 /** Initialize the DataLink configuration from Environment variables,
  * or else to defaults.
  * @ingroup DataLink

src/bacnet/datalink/dlenv.h

@@ -43,10 +43,24 @@ extern "C" {
     int dlenv_register_as_foreign_device(
         void);
 
+    /* Non-blocking counterpart -- see its comment in dlenv.c. Pair with
+     * dlenv_maintenance_timer_async() below for periodic renewal too. */
+    BACNET_STACK_EXPORT
+    int dlenv_register_as_foreign_device_async(
+        void);
+
     BACNET_STACK_EXPORT
     void dlenv_maintenance_timer(
         uint16_t elapsed_seconds);
 
+    /* Non-blocking counterpart -- see its comment in dlenv.c. Call this
+     * instead of dlenv_maintenance_timer() above, not in addition to
+     * it, for a caller using the async registration API. Same return
+     * contract as dlenv_register_as_foreign_device_async(). */
+    BACNET_STACK_EXPORT
+    int dlenv_maintenance_timer_async(
+        uint16_t elapsed_seconds);
+
     BACNET_STACK_EXPORT
     void dlenv_bbmd_address_set(
         BACNET_IP_ADDRESS *address);