Update project

.gitmodules

@@ -1,3 +1,4 @@
 [submodule "vendor/libplum"]
 	path = vendor/libplum
-	url = file:///home/arnaud/Work/2-towns/libplum
+	url = https://github.com/2-towns/libplum.git
+	branch = feat/configurable-timeouts

README.md

@@ -6,20 +6,6 @@ libplum tries each protocol in order (PCP → NAT-PMP → UPnP-IGD) and falls ba
 
 ## Installation
 
-Clone with submodules:
-
-```bash
-git clone --recurse-submodules <REPO_URL>
-```
-
-Or after cloning:
-
-```bash
-git submodule update --init --recursive
-```
-
-Install via Nimble:
-
 ```bash
 nimble install
 ```
@@ -31,11 +17,15 @@ import chronos
 import libplum/plum
 
 proc main() {.async.} =
-  check init().isOk()
+  let initRes = init()
+  if initRes.isErr():
+    echo "init failed: ", initRes.error
+    return
 
   let r = await createMapping(TCP, 8080)
   if r.isErr():
     echo "failed: ", r.error
+    discard cleanup()
     return
 
   let res = r.value
@@ -47,7 +37,25 @@ proc main() {.async.} =
 waitFor main()
 ```
 
-See the [examples](examples) directory for a complete example.
+See [examples/port_mapping.nim](examples/port_mapping.nim) for a complete example that pauses between add and remove so you can verify the mapping on your router:
+
+```bash
+nim c -r examples/port_mapping.nim
+```
+
+### Timeouts
+
+Pass timeout options to `init` to control how long discovery and mapping wait:
+
+```nim
+discard init(discoverTimeout = 5000, mappingTimeout = 10000)
+```
+
+Pass a `timeout` to `createMapping` to control the overall wait:
+
+```nim
+let r = await createMapping(TCP, 8080, timeout = seconds(15))
+```
 
 ### Ongoing state changes
 
@@ -60,16 +68,29 @@ proc onStateChange(state: PlumState, mapping: PlumMapping) {.cdecl, raises: [],
 let r = await createMapping(TCP, 8080, onStateChange = onStateChange)
 ```
 
-### Configurable timeouts
-
-The discovery and mapping timeouts can be configured via `init`:
+### Checking mapping state
 
 ```nim
-discard init(
-  discoverTimeout = 5000,  # ms, default 10000
-  mappingTimeout  = 5000,  # ms, default 10000
-  recheckPeriod   = 60000, # ms, default 300000
-)
+if hasMapping(id):
+  echo "mapping is still active"
+```
+
+## API
+
+See [api.md](api.md) for the full API reference.
+
+## Testing
+
+Basic tests run without a router:
+
+```bash
+nimble test
+```
+
+To run tests against a real NAT device:
+
+```bash
+NAT_TEST_PLUM=1 nimble test
 ```
 
 ## License

api.md

@@ -0,0 +1,113 @@
+# API Reference
+
+## Types
+
+```nim
+type PlumProtocol* = enum
+  TCP
+  UDP
+```
+
+```nim
+type PlumState* = enum
+  Destroyed
+  Pending
+  Success
+  Failure
+  Destroying
+```
+
+```nim
+type PlumMapping* = object
+  protocol*: PlumProtocol
+  internalPort*: uint16
+  externalPort*: uint16
+  externalHost*: string
+```
+
+```nim
+type MappingResult* = object
+  id*: cint
+  mapping*: PlumMapping
+```
+
+```nim
+type PlumStateCallback* =
+  proc(state: PlumState, mapping: PlumMapping) {.cdecl, raises: [], gcsafe.}
+```
+
+## Procedures
+
+### init
+
+```nim
+proc init*(
+    logLevel: plum_log_level_t = PLUM_LOG_LEVEL_NONE,
+    discoverTimeout: int = 0,
+    mappingTimeout: int = 0,
+    recheckPeriod: int = 0
+): Result[void, string]
+```
+
+Initializes the library and starts the internal thread. Must be called before any other proc.
+
+- `logLevel`: verbosity of internal logs (default: none)
+- `discoverTimeout`: how long to probe for a NAT device, in ms (default: 10000)
+- `mappingTimeout`: how long to wait for a mapping response, in ms (default: 10000)
+- `recheckPeriod`: interval between periodic mapping rechecks, in ms (default: 300000)
+
+### cleanup
+
+```nim
+proc cleanup*(): Result[void, string]
+```
+
+Stops the internal thread and releases all resources. Returns an error if called more than once or before `init`.
+
+### createMapping
+
+```nim
+proc createMapping*(
+    protocol: PlumProtocol,
+    internalPort: uint16,
+    externalPort: uint16 = 0,
+    timeout: Duration = seconds(30),
+    onStateChange: PlumStateCallback = nil
+): Future[Result[MappingResult, string]]
+```
+
+Requests a port mapping from the NAT device. Tries PCP, then NAT-PMP, then UPnP-IGD in order.
+
+- `protocol`: `TCP` or `UDP`
+- `internalPort`: the local port to map
+- `externalPort`: preferred external port, or 0 to let the router choose
+- `timeout`: how long to wait for the mapping to be established (default: 30s)
+- `onStateChange`: optional callback invoked when the mapping state changes after the initial result
+
+Returns a `MappingResult` containing the mapping `id` (needed for `destroyMapping`) and the `PlumMapping` with the external address and port.
+
+Returns an error if no NAT device is found, the mapping fails, or the timeout expires.
+
+### destroyMapping
+
+```nim
+proc destroyMapping*(id: cint)
+```
+
+Removes a mapping. Must be called exactly once after a successful `createMapping`.
+
+### hasMapping
+
+```nim
+proc hasMapping*(id: cint): bool
+```
+
+Returns true if the mapping exists and has not been destroyed yet.
+
+### getLocalAddress
+
+```nim
+proc getLocalAddress*(): Result[string, string]
+```
+
+Returns the local IP address as seen from the network interface. Useful to check whether the machine already has a public IP (in which case no NAT mapping is needed).

config.nims

@@ -1,7 +1,6 @@
 switch("threads", "on")
 
 # begin Nimble config (version 2)
---noNimblePath
 when withDir(thisDir(), system.fileExists("nimble.paths")):
   include "nimble.paths"
 # end Nimble config

examples/port_mapping.nim

@@ -7,7 +7,8 @@
 # those terms.
 
 # This example creates a TCP port mapping via PCP, NAT-PMP, or UPnP-IGD,
-# prints the external address, and destroys the mapping before exiting.
+# prints the external address so you can verify it on the router,
+# then waits for Enter before removing the mapping.
 
 import chronos
 import libplum/plum
@@ -20,6 +21,7 @@ proc main() {.async.} =
 
   echo "Local address: ", getLocalAddress().valueOr("<unknown>")
 
+  # --- Add mapping ---
   let r = await createMapping(TCP, 8080)
   if r.isErr():
     echo "createMapping failed: ", r.error
@@ -28,11 +30,16 @@ proc main() {.async.} =
 
   let res = r.value
   echo "Mapping created:"
-  echo "  external host: ", res.mapping.externalHost
-  echo "  external port: ", res.mapping.externalPort
+  echo "  external: ", res.mapping.externalHost, ":", res.mapping.externalPort
   echo "  internal port: ", res.mapping.internalPort
+  echo ""
+  echo "Verify the mapping on your router, then press Enter to remove it."
+  try: discard readLine(stdin)
+  except IOError: discard
 
+  # --- Remove mapping ---
   destroyMapping(res.id)
+  echo "Mapping removed."
 
   let cleanupRes = cleanup()
   if cleanupRes.isErr():

libplum.nimble

@@ -10,7 +10,8 @@ installDirs   = @["libplum", "vendor"]
 ### Dependencies
 requires "nim >= 1.6.0",
          "results >= 0.4.0",
-         "chronos >= 4.2.0 & < 5.0.0"
+         "chronos >= 4.2.0 & < 5.0.0",
+         "unittest2"
 
 proc compileStaticLibraries() =
   withDir "vendor/libplum":
@@ -22,6 +23,7 @@ task buildBundledLibs, "build bundled libraries":
 
 task test, "run tests":
   compileStaticLibraries()
+  exec("nimble setup")
   exec("nim c -r tests/test_plum.nim")
 
 before install:

libplum/libplum.nim

@@ -66,9 +66,12 @@ type
 
   # Define the config struct, passed by copy (usual for struct).
   plum_config_t* {.importc: "plum_config_t", header: "plum.h", bycopy.} = object
-    log_level*    {.importc: "log_level".}:    plum_log_level_t
-    log_callback* {.importc: "log_callback".}: plum_log_callback_t
-    dummytls_domain* {.importc: "dummytls_domain".}: cstring
+    log_level*        {.importc: "log_level".}:        plum_log_level_t
+    log_callback*     {.importc: "log_callback".}:     plum_log_callback_t
+    dummytls_domain*  {.importc: "dummytls_domain".}:  cstring
+    discover_timeout* {.importc: "discover_timeout".}: cint # msecs, 0 means use default (10000)
+    mapping_timeout*  {.importc: "mapping_timeout".}:  cint # msecs, 0 means use default (10000)
+    recheck_period*   {.importc: "recheck_period".}:   cint # msecs, 0 means use default (300000)
 
   # Define the mapping struct, passed by copy (usual for struct).
   # The user_ptr is a pointer to the MappingHandle in order to receive the result

libplum/plum.nim

@@ -103,7 +103,7 @@ proc mappingCallback(id: cint, state: plum_state_t,
       protocol: PlumProtocol(raw[].protocol.int),
       internalPort: raw[].internal_port,
       externalPort: raw[].external_port,
-      externalHost: $cast[cstring](unsafeAddr raw[].external_host[0])
+      externalHost: $cast[cstring](addr raw[].external_host)
     )
 
     if not handle.resolved.exchange(true):
@@ -118,13 +118,21 @@ proc mappingCallback(id: cint, state: plum_state_t,
       if not handle.onStateChange.isNil:
         handle.onStateChange(plumState, mapping)
 
-proc init*(logLevel = PLUM_LOG_LEVEL_NONE): Result[void, string] {.raises: [].} =
+proc init*(
+    logLevel: plum_log_level_t = PLUM_LOG_LEVEL_NONE,
+    discoverTimeout: int = 0,
+    mappingTimeout: int = 0,
+    recheckPeriod: int = 0
+): Result[void, string] {.raises: [].} =
   ## init MUST be called to setup internal plum thread (plum_init).
 
   var config = plum_config_t(
     log_level: logLevel,
     log_callback: nil,
-    dummytls_domain: nil
+    dummytls_domain: nil,
+    discover_timeout: discoverTimeout.cint,
+    mapping_timeout: mappingTimeout.cint,
+    recheck_period: recheckPeriod.cint
   )
 
   let res = plum_init(addr config)
@@ -146,6 +154,7 @@ proc createMapping*(
     protocol: PlumProtocol,
     internalPort: uint16,
     externalPort: uint16 = 0,
+    timeout: Duration = seconds(30),
     onStateChange: PlumStateCallback = nil
 ): Future[Result[MappingResult, string]] {.async: (raises: [CancelledError]).} =
   let signal = ThreadSignalPtr.new().valueOr:
@@ -174,24 +183,23 @@ proc createMapping*(
   var completed = false
   try:
     # Wait for the callback to fireSync
-    completed = await withTimeout(signal.wait(), seconds(30))
+    completed = await withTimeout(signal.wait(), timeout)
   except CancelledError:
+    # CancelledError skips the lines below and propagates after finally.
     raise
   finally:
     if not completed:
-      # The signal reached timeout or was cancelled,
-      # we cannot close it now otherwise the pending operation
-      # might trigger a memory issue.
-      # When entering into DESTROYING state, libplum will ignore
-      # the pending operation so closing the signal in the DESTROYED
-      # callback is safe.
+      # Timeout or cancellation: we cannot close the signal here because
+      # the C callback may fire later and call fireSync on it.
+      # Mark the handle as abandoned so the DESTROYED callback closes it.
       handle.abandoned.store(true)
       discard plum_destroy_mapping(id)
     else:
-      # The signal is completed, we can close it
+      # Signal fired normally, safe to close.
       discard signal.close()
-      
-  if not completed:  
+
+  # Reached only when completed = true (CancelledError skips this).
+  if not completed:
     return err("plum: mapping " & $id & " timed out")
 
   if handle.resolvedState == Success:
@@ -204,11 +212,16 @@ proc destroyMapping*(id: cint) {.raises: [].} =
   ## Must be called exactly once after a successful createMapping.
   discard plum_destroy_mapping(id)
 
+proc hasMapping*(id: cint): bool {.raises: [].} =
+  ## Returns true if the mapping exists and has not been destroyed yet.
+  withSafeLock:
+    result = id in activeMappings
+
 proc getLocalAddress*(): Result[string, string] {.raises: [].} =
   var buf = newString(PLUM_MAX_ADDRESS_LEN)
   let res = plum_get_local_address(buf.cstring, buf.len.csize_t)
   if res >= 0:
-    buf.setLen(min(res, PLUM_MAX_ADDRESS_LEN))
+    buf.setLen(min(res.int, PLUM_MAX_ADDRESS_LEN))
     ok(buf)
   else:
     err("plum_get_local_address failed: " & $res)

tests/nim.cfg

@@ -0,0 +1 @@
+-p:".."

tests/test_plum.nim

@@ -31,10 +31,17 @@ suite "plum":
     check r.value.len > 0
     discard cleanup()
 
+  test "hasMapping returns false for unknown id":
+    check not hasMapping(999)
+
   test "createMapping fails without router":
     # In CI with no NAT device, expect Failure or timeout — both return err.
+    if getEnv("NAT_TEST_PLUM") == "1":
+      skip()
+      return
+
     discard init()
-    let r = waitFor createMapping(UDP, 12345)
+    let r = waitFor createMapping(UDP, 12345, timeout = seconds(5))
     check r.isErr()
     discard cleanup()
 
@@ -44,30 +51,31 @@ suite "plum - NAT port mapping (requires NAT_TEST_PLUM=1)":
       skip()
       return
 
-    check init().isOk()
+    check init(discoverTimeout = 15000).isOk()
 
-    let r = waitFor createMapping(TCP, 8101)
+    let r = waitFor createMapping(TCP, 8101, timeout = seconds(40))
     check r.isOk()
+    if r.isOk():
+      let res = r.value
+      check res.mapping.externalPort > 0
+      check res.mapping.externalHost.len > 0
+      check hasMapping(res.id)
+      destroyMapping(res.id)
 
-    let res = r.value
-    check res.mapping.externalPort > 0
-    check res.mapping.externalHost.len > 0
-
-    destroyMapping(res.id)
-    check cleanup().isOk()
+    discard cleanup()
 
   test "createMapping UDP":
     if getEnv("NAT_TEST_PLUM") != "1":
       skip()
       return
 
-    check init().isOk()
+    check init(discoverTimeout = 2000).isOk()
 
-    let r = waitFor createMapping(UDP, 8090)
+    let r = waitFor createMapping(UDP, 8090, timeout = seconds(40))
     check r.isOk()
+    if r.isOk():
+      let res = r.value
+      check res.mapping.externalPort > 0
+      destroyMapping(res.id)
 
-    let res = r.value
-    check res.mapping.externalPort > 0
-
-    destroyMapping(res.id)
-    check cleanup().isOk()
+    discard cleanup()

vendor/libplum

@@ -1 +1 @@
-Subproject commit ff91874909882e0ba708a84cd9f1cf5761631869
+Subproject commit 7b3639bcf973ff9e3de554545f703f5a595a0536