Skip to content

ci: add GitHub Actions workflow with smoke + bacnet-sim-ci integration#44

Open
ravz wants to merge 13 commits into
masterfrom
ci/bacnet-sim-integration
Open

ci: add GitHub Actions workflow with smoke + bacnet-sim-ci integration#44
ravz wants to merge 13 commits into
masterfrom
ci/bacnet-sim-integration

Conversation

@ravz
Copy link
Copy Markdown
Collaborator

@ravz ravz commented Apr 30, 2026

First CI for this repo. Both jobs green.

Jobs

`smoke`

  • `node --check` on every committed `.js` file (excluding the vendored bacstack dist).
  • Smoke that `require`s the five non-Node-RED-factory modules and asserts their public shape: `BacnetClient`, `BacnetServer`, `BacnetDevice`, `treeBuilder`, `common` — 16/16 pass.
  • The Node-RED factory modules (`bacnet_gateway`, `bacnet_read`, `bacnet_write`, `bacnet_inspector`, `bitpool_inject`) export `function(RED)` and can't be exercised without a fake RED runtime — they get syntax coverage only.

`integration` (real end-to-end against bacnet-sim-ci)

  • Custom Docker network (`172.20.0.0/24`) with explicit container IPs:
    • `bacnet-sim` at `172.20.0.10` — runs the bacnet-sim-ci image with deviceId 1001
    • `nodered` at `172.20.0.20` — runs Node-RED with this branch's edge-bacnet installed as a palette (built from local source via `npm install --install-links /tmp/edge-bacnet` so deps resolve into `/data/node_modules`)
  • Pre-loaded `flows.json` with a single `Bacnet-Gateway` (`toLogIam: true`, 5-second discover schedule, broadcast `172.20.0.255`).
  • Assertion: grep node-red logs for `BACnet device found: 1001` within 90s.
  • Currently passes in 7s.

This proves edge-bacnet end-to-end:

  • bound a UDP socket
  • issued a global Who-Is broadcast on the test subnet
  • received an I-Am from the simulated device
  • parsed and registered the device

Local dev

```bash
npm run test:unit # smoke (no network)
```

To exercise the integration test locally, mirror the workflow:
```bash
docker network create --subnet=172.20.0.0/24 bacnet-test-net
docker run -d --name bacnet-sim --network bacnet-test-net --ip 172.20.0.10
--cap-add=NET_ADMIN -e BACNET_DEVICE_ID=1001 -e BACNET_DEVICE_NAME=TestDevice
ghcr.io/rise-building-technology/bacnet-sim-ci:latest
docker build -f tests/integration/Dockerfile -t nodered-edgebacnet:test .
docker run -d --name nodered --network bacnet-test-net --ip 172.20.0.20 nodered-edgebacnet:test
docker logs -f nodered | grep "BACnet device found"
```

Conventions

  • Tests live under `tests/` (plural) — `test/` (singular) is in `.gitignore` from an existing convention.
  • New files added: `.dockerignore`, `.github/workflows/ci.yml`, `tests/unit/smoke.js`, `tests/integration/Dockerfile`, `tests/integration/flows.json`.
  • No new runtime dependencies. Tests use the vendored bacstack and core Node modules; the integration job's only "deps" are Docker images.

Adds the first CI for this repo. Two jobs, both on ubuntu-latest:

* `smoke` — runs `node --check` on every committed .js file (excluding
  the vendored bacstack dist) and a Node smoke test that requires the
  five non-Node-RED-factory modules and asserts their public shape
  (BacnetClient, BacnetServer, BacnetDevice, treeBuilder, common). The
  Node-RED factory modules (gateway/read/write/inspector/inject) export
  `function(RED)` and can't be exercised without a fake RED runtime;
  they get syntax coverage only.

* `integration` — runs bacnet-sim-ci (a multi-device BACnet/IP
  simulator) as a service container with `--cap-add=NET_ADMIN` and
  performs a real BACnet readProperty round-trip against the simulated
  device's PRESENT_VALUE, asserting it matches the sim's REST view.
  Uses unicast (target IP from the sim's REST API) rather than
  Who-Is/I-Am broadcast — UDP broadcast is unreliable across Docker
  network boundaries and the broadcast path can be added separately
  once the unicast baseline is green.

Test scripts:
- `npm run test:unit`        - smoke (no network)
- `npm run test:integration` - readProperty against sim (needs sim running)

The integration test is also runnable locally against any
bacnet-sim-ci container the developer starts (env vars: SIM_API_URL,
SIM_BACNET_PORT, LOCAL_BACNET_PORT, READY_TIMEOUT_MS).

Tests live under `tests/` (plural) because `test/` (singular) is in
.gitignore as part of an existing convention.
Copilot AI review requested due to automatic review settings April 30, 2026 05:37
Two issues from the first CI run:

1. REST shape: the sim returns {presentValue, ...} not {value, ...} for
   GET /api/devices/{id}/objects/{type}/{instance}. Read presentValue.

2. BACnet UDP could not reach the sim from the runner host: the bacstack
   readProperty timed out (ERR_TIMEOUT after 6s). Per the sim's docs the
   test client must be on the same Docker network — UDP broadcast and
   even unicast through docker-proxy to a service container is unreliable.

   Fix: run the integration job inside `container: node:20-bookworm` so
   it joins the same user-defined Docker network as the bacnet-sim
   service. The sim is now reachable by service hostname (bacnet-sim)
   and BACnet UDP routes directly across the bridge instead of through
   docker-proxy. Drop the host port mappings (no longer needed) and
   point SIM_API_URL at http://bacnet-sim:8099 via env.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds the repo’s first CI pipeline by introducing lightweight module-load smoke tests plus an end-to-end BACnet/IP readProperty integration test against a bacnet-sim-ci service container, and wires them into npm scripts and a GitHub Actions workflow.

Changes:

  • Add tests/unit/smoke.js to validate key modules can be require()’d and expose expected public shapes.
  • Add tests/integration/sim-readproperty.js to perform a real BACnet readProperty round-trip against bacnet-sim-ci, cross-checking with the sim’s REST API.
  • Add CI workflow and npm scripts to run smoke + integration in GitHub Actions.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
tests/unit/smoke.js New smoke test to load core modules and assert expected exports/prototypes.
tests/integration/sim-readproperty.js New integration test that polls sim readiness, fetches device info via REST, and performs BACnet readProperty.
package.json Adds test:unit and test:integration scripts for CI/local execution.
.github/workflows/ci.yml Introduces GitHub Actions workflow with smoke and integration jobs (service container + failure logs).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/ci.yml
Comment thread .github/workflows/ci.yml Outdated
ravz added 5 commits April 30, 2026 13:42
- The previous "REST shape" change was a missed Edit (file-not-read
  error) and didn't ship. Now read presentValue, not value.
- Bind the test client to 47808 (the standard BACnet port). Inside the
  test container nothing else binds it, and some BACnet servers only
  reliably respond when the request comes from the standard port.
- Add a 500ms delay after bacstack Client construction to let the
  underlying dgram socket finish binding before the first send.
The services:+container: approach didn't get BACnet UDP through. Switch
to the action-wrapper pattern documented in the sim's own examples/:
runs-on host with the sim started by the action, address localhost via
port-forwarding. Test binds to 47809 since 47808 on the host is taken
by docker-proxy.
The sim starts and is reachable via REST, but BACnet readProperty
times out across the docker-bridge boundary in all three topologies
tried on this branch (services+host, services+container,
docker-run+host). Real bug worth fixing, but needs hands-on
packet-capture debugging in a runner shell — not push-watch
iteration. Land smoke + scaffolding now; iterate on integration in
a follow-up.
@ravz
Copy link
Copy Markdown
Collaborator Author

ravz commented Apr 30, 2026

Workflow is green at the job-policy level: `Syntax + module load` passes (16/16 assertions), and the integration job is marked `continue-on-error: true` so it doesn't block. The integration check will appear red on the PR until the BACnet UDP path is debugged — that's intentional, kept visible as a TODO marker rather than hidden.

What's known about the integration failure:

  • Sim starts cleanly; `Confirm sim devices are visible` step shows the device JSON.
  • REST round-trip works: `ok sim REST API is ready`, `ok sim returns at least one device`, `ok REST GET analog-input/1 returns a numeric presentValue`.
  • BACnet UDP `readProperty` to the sim's container IP (`172.17.0.2:47808`) times out at 6s.
  • Sim logs confirm BAC0 is bound to `172.17.0.2/24` on port 47808 and serving REST. No "received" log entry for the BACnet attempt — packets aren't reaching the application.

What I tried:

  1. `services:` + runs-on host (UDP via docker-proxy)
  2. `services:` + `container: node:20-bookworm` job (same Docker network, addressing by service hostname)
  3. `docker run` + runs-on host (mirrors the sim's own `action.yml`)

All three lose the response packet between the sim's container and the test client.

Most likely root cause (uneliminated): the bacstack client binds to `0.0.0.0:47808/47809` and sends the request, but BAC0/BACpypes3 may only respond when the request arrives on its bound address (172.17.0.2). Through docker-proxy NAT the request appears to arrive there, but the response back through the bridge to the runner host's bridge IP (172.17.0.1) might be filtered or rewritten in a way that bacstack doesn't recognize as matching its outstanding invokeId.

Best next step: add a `tcpdump -i any -nn 'udp port 47808'` step alongside the test in CI to confirm whether the response packet is sent and where it goes. That's a 5-minute change but better done with a maintainer's eye on the result. Happy to push it as a follow-up if you'd like.

ravz added 6 commits April 30, 2026 14:27
The new integration job actually exercises this branch's code:

  bacnet-sim (172.20.0.10) <-- BACnet/IP --> Node-RED + edge-bacnet (172.20.0.20)

Both run as peer containers on a custom Docker network (subnet
172.20.0.0/24) — pattern from rise-building-technology/bacnet-sim-ci-test.
No docker-proxy NAT, no runner-host bridge IP. The earlier
services:/host-runner/docker-run-on-host topologies all lost the BACnet
UDP response across the docker-bridge boundary; this matches the sim
maintainer's blessed setup and avoids that class of failure.

The Node-RED image (tests/integration/Dockerfile) installs
@bitpoolos/edge-bacnet from THIS branch's local source, so the test
exercises in-tree changes (not whatever's published on npm). The
pre-loaded flow (tests/integration/flows.json) has a single
Bacnet-Gateway with toLogIam=true and a 5-second discover schedule.

Assertion: after starting both containers, grep node-red logs for
"BACnet device found: 1001" within 90s. That single log line proves
edge-bacnet successfully:
  - bound a UDP socket
  - issued a global Who-Is broadcast
  - received an I-Am from the sim
  - parsed the I-Am and registered the device

Drops the prior bacstack-direct test (tests/integration/sim-readproperty.js)
and the test:integration npm script — those proved the bacstack
primitives but didn't exercise edge-bacnet's wrapper, and never got
past the network topology issues.

Adds a top-level .dockerignore so the Node-RED image build doesn't
copy node_modules / .git / docs into the layer.
Previous build ran 'npm install /tmp/edge-bacnet' from /usr/src/node-red,
which produced 'Cannot find module toad-scheduler' at runtime — the
standard Node-RED palette directory is /data/node_modules and the
node-red user has write perms there. Also drop --omit=dev (was masking
some runtime deps in this docker context).
Gateway uses these as filters; without them scanMatrix is empty and
discovered devices may be silently dropped. Match the values from the
project's own examples/2-Discover-Write.json (full BACnet device-id
range, port 47808 only).
Captured digests from the green run on this branch:
- ghcr.io/rise-building-technology/bacnet-sim-ci@sha256:29ed481a...
- nodered/node-red:latest@sha256:a5cb1dcd...

Bump after verifying newer upstream images work locally. Addresses
Copilot review feedback on PR #44.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants