[DeathCamel58]

gpio_write() (the sysfs path, kernels older than 4.8) exports the pin, writes the value, then unexports it. On kernels where freeing the line makes the pin controller turn it back into an input (seen on Ambarella S3L, Linux 3.10), the level is lost right after the write: IR LEDs switch off immediately and IR-cut pulses don't hold, while /api/night still reports irled: true.

This leaves the pin exported after a write, and only exports it when its sysfs node isn't there yet, so later writes don't log "Unable to export".

Tested on an Amcrest IP4M-1026E (S3L, OV4689): with night mode on, GPIO 5 stays out hi in /sys/kernel/debug/gpio and the IR LEDs stay lit. Without the change they dropped to input right after "on". Not run on other platforms; gpio_read() still unexports.

The same text is saved in logs/pr-gpio-keep-exported.md.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Keep legacy sysfs GPIO outputs exported until no longer needed

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Keep sysfs GPIO outputs exported after writes so older kernels retain their output levels.
• Release night-mode pins when disabled or reassigned, without dropping outputs during thread
 restarts.
• Test first writes, repeated writes, release, and both standard and Fullhan sysfs node names.

Diagram

graph TD
  API["Night API"] -->|restart| Night["Night thread"] -->|write outputs| HAL["GPIO HAL"] -->|export and set| Sysfs["GPIO sysfs"]
  API -->|release unused| HAL
  HAL -->|unexport retired| Sysfs

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Own pin reconciliation within night control

• ➕ Keeps GPIO resource ownership alongside night-mode output control.
• ➕ Could cover future night-mode configuration paths without API-specific cleanup.
• ➖ Requires a broader lifecycle refactor and coordination with thread shutdown and restart.

Recommendation: Use the PR's targeted release on night API changes: it addresses stale exports without releasing outputs on every thread restart. Moving pin reconciliation into night control is worth considering if additional configuration paths begin changing these pins.

Files changed (5) +174 / -12

Enhancement (1) +1 / -0

gpio.hExpose GPIO pin release +1/-0

Expose GPIO pin release

• Declares gpio_release for callers that stop using a previously written output pin.

src/hal/gpio.h

Bug fix (2) +38 / -12

gpio.cRetain legacy sysfs output exports and add explicit release +29/-12

Retain legacy sysfs output exports and add explicit release

• Sysfs writes export only missing pins and leave them exported to preserve output levels. Node-name detection now caches a format only after a node exists, supporting Fullhan uppercase names; gpio_release unexports an existing node, while the newer GPIO path provides a no-op implementation. The sysfs root is configurable for testing.

src/hal/gpio.c

server.cRelease retired night-mode GPIO pins +9/-0

Release retired night-mode GPIO pins

• The night API compares output pins before and after a configuration change and releases valid old pins that are no longer used or whose night mode was disabled. Release occurs after stopping the night thread and before restarting it.

src/server.c

Tests (2) +135 / -0

gpio_test.cExercise legacy GPIO writes against fake sysfs +123/-0

Exercise legacy GPIO writes against fake sysfs

• A fake export/unexport interface tests first and repeated writes, retained exports, and explicit release. Separate runs cover lowercase standard nodes and uppercase Fullhan nodes.

tst/gpio-sysfs/gpio_test.c

run.shRun both sysfs GPIO node-name test variants +12/-0

Run both sysfs GPIO node-name test variants

• Compiles the fake-sysfs GPIO test and runs it in separate processes for lowercase and uppercase node naming, so each run tests format detection independently.

tst/gpio-sysfs/run.sh

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Clients can release another GPIO user's pin ✓ Resolved

Resolving commit: 2d9b6ad

Resolution reason

Ownership is now tracked with a bitmap (gpio_owned) of pins this process exported. gpio_write() sets
the bit only when its own gpio_export(pin, true) succeeds; gpio_read/gpio_write disown on unexport.
gpio_release() returns early unless GPIO_OWNED(pin) is set, so it never unexports pins exported by
others. gpio_release_unused() only releases owned pins not in the keep list, and server.c calls it
after night-mode reconfiguration. The test covers the case of a pre-existing node being left alone.

Description

gpio_release() treats the existence of a sysfs node as sufficient reason to unexport it, and
/api/night calls it for former assignments without checking whether this process exported them.
With night mode off, two pin-assignment requests can designate and then release a GPIO held by
another component; the endpoint is unauthenticated in the shipped configuration.

---

2. One night request leaves a new LED lit ✓ Resolved

Resolving commit: 2d9b6ad

Resolution reason

The commit makes gpio_write() record pins it exported itself (GPIO_OWN), and adds
gpio_release()/gpio_release_unused(). In server.c, after night_disable(), the request handler calls
gpio_release_unused(keep, kept) with the pins night mode still drives (night_pins()), so any pin
exported during the request (including a newly assigned irled pin written while night mode is off)
is unexported unless still in use. This releases the leaked LED pin regardless of whether it was in
oldPins, though via a different mechanism than the suggested per-request tracking of pins; it
addresses the finding's concern.

Description

The /api/night cleanup iterates only oldPins, although query parameters can change an assignment
and call night_irled() before cleanup runs. A request such as irled_pin=5&irled=true&enable=0
writes GPIO 5 while night mode is off, but never releases it if 5 was not assigned before the
request.

---

3. First GPIO writes fail on Fullhan kernels ✓ Resolved

Description

gpio_write() now calls gpio_node() before exporting an unexported pin, causing its process-wide
format cache to select the lowercase path. On Fullhan kernels, export creates an uppercase GPIO
directory, so the following direction change uses a nonexistent path and the write fails.

---

4. Fullhan's white-lamp pin stays exported ✓ Resolved

Resolving commit: 2d9b6ad

Resolution reason

night_pins() now includes FH_LAMP_WHITE_GPIO when plat == HAL_PLATFORM_F1 on the ARM v6 build
(src/night.c), and server.c's /api/night handler calls gpio_release_unused(keep, kept) with that
list, so the white-lamp pin is released unless it is still in use by night mode. Also gpio_write()
now records exported pins as owned so only pins this process exported are unexported. The fix is in
a different structure than the suggested fix (a keep-list helper rather than adding the pin to the
oldPins/newPins comparison), but it covers the same lamp pin on the applicable platform.

Description

The /api/night release loop tracks only the configured IR-cut and IR-LED pins, but
fh_lamp_enable() also writes fixed GPIO 11. On a Fullhan F1 using the sysfs path, disabling night
mode leaves that lamp-control pin exported unless it happens to share a configured output
assignment.

---

5. Normal shutdown leaves night pins exported ✓ Resolved

Resolving commit: 2d9b6ad

Resolution reason

The commit adds gpio_release_unused() and calls it from main.c right after night_disable() on the
normal shutdown path (gpio_release_unused(NULL, 0) releases every pin this process exported).
Ownership tracking via gpio_owned ensures only pins this process exported are unexported. This
addresses the leak on normal shutdown, though via a different mechanism than the suggested
per-shutdown cleanup after the night thread stops (the call is placed before sdk_stop(), not after
the night thread stops).

Description

The sysfs gpio_write() no longer unexports its output, while the sysfs gpio_deinit() is empty
and normal shutdown calls only night_disable(). If night mode has written configured outputs,
stopping the process leaves those lines exported because the new release loop runs only for
/api/night requests.

---

6. Disabled night mode leaves old pins active ✓ Resolved

Description

gpio_write() now leaves output pins exported, but neither night_disable() nor the sysfs
gpio_deinit() releases them. When /api/night disables night mode or changes an output pin
assignment, the old pin remains exported at its last output level after the night thread stops.

---

Tip of the day

💡 Did you know, you can see how risky a PR is at a glance with blast radius labels

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[DeathCamel58]

I pushed a second commit, dbd2a80. It answers the two findings from the automated review of the first commit. I don't expect that review to run again, so here is how each is handled, and one thing I can't test myself.

1. First write on a Fullhan kernel (the node name was cached too early). Right: with the new "export only if the node is missing" check, gpio_node() asked about the pin before it was exported, found neither name, and cached the lowercase gpio<n>. On Fullhan the export then creates GPIO<n>, so the direction write used a path that didn't exist. Now the name format is cached only once a node actually exists, and a pin with no node yet is exported first and named afterwards.

2. Pins stay exported after night mode is turned off or a pin is moved. Fixed in /api/night: a pin that is no longer in use is released through a new gpio_release(), which unexports it. I did not put this in gpio_deinit() on purpose. The night thread calls it every time it stops, and every /api/night request restarts that thread, so releasing there would drop the IR LEDs on each request. A pin still stays exported when the whole process exits; gpio_read() still unexports, unchanged.

Tests. tst/gpio-sysfs (run by ./test.sh) runs the sysfs path against a fake sysfs in a temporary directory, once for each node name, and covers the first write to an unexported pin, no re-export on later writes, and release. It fails on the first commit for the Fullhan name. On an Amcrest IP4M-1026E (Ambarella S3L, Linux 3.10, lowercase gpio<n>), pin 5 stays out hi while night mode is on, is released by /api/night?enable=0 and when irled_pin is moved, and comes back when night mode is re-enabled.

Could someone with a Fullhan camera test this? I don't have one, so the GPIO<n> path is covered only by the fake sysfs. It needs a Fullhan camera whose kernel uses the old sysfs GPIO interface (before 4.8) and an IR LED or IR-cut on a GPIO:

1. Make sure the pin isn't exported at start (ls /sys/class/gpio shows no node for it; reboot if another streamer used it).
2. Start Divinus with night_mode enabled and ir_led_pin set to that pin.
3. curl 'http://CAMERA/api/night?irled=1'. Expect no "Unable to control the direction of GPIO pin" line in the log, a /sys/class/gpio/GPIO<n> node afterwards with direction = out and value = 1, and the LED lit. Then irled=0 and irled=1 again: no "Unable to export" lines.
4. curl 'http://CAMERA/api/night?enable=0': the node should disappear. enable=1: it should come back.

Please report the SoC, uname -r, the exact node name you see, and any gpio lines from the log. On the first commit, step 3 should fail with the direction error, if you want to confirm the bug as well.

[DeathCamel58]

Thanks for the re-review. The first two findings are marked resolved. It found four more, all about the new gpio_release(), and they share one cause: nothing recorded who exported a pin. A third commit, 2d9b6ad, fixes that.

gpio_write() now records the pins it exports itself, and only those are ever unexported. gpio_release() ignores any other pin, and a new gpio_release_unused() lets go of every recorded pin that is not in a list of pins still in use.

Clients can release another GPIO user's pin. A pin this process did not export is never unexported, so moving irled_pin to someone else's pin and away again does nothing to it.

One night request leaves a new LED lit (irled_pin=5&irled=true&enable=0). /api/night now releases every recorded pin that night mode no longer drives, not just the pins it had before the request, so a pin assigned and driven within one request is let go of.

The Fullhan white-lamp pin stays exported. The new night_pins() lists the pins night mode drives, and on the F1 that includes the white-lamp GPIO 11. It stays exported while night mode is on and is released when night mode is switched off. I can't run this on a Fullhan camera; it is covered by reading the code and the fake sysfs.

Normal shutdown leaves night pins exported. main() now releases whatever is still recorded after the night thread stops. gpio_deinit() still does not, because the night thread calls it every time /api/night restarts it. This changes one sentence of my last comment: a pin no longer stays exported when the process exits.