New Contributor: Seeking Guidance on High-Value, Low-Noise PRs

[tehranian]

Greetings! I recently bought a pool with Jandy equipment and purchased your CM module so I can control it through Apple HomeKit / Home Assistant.

AqualinkD is exactly the kind of project I want to support, and I’d like to invest time contributing. Before I open PRs, I wanted to ask for guidance so I don’t create noisy or unwanted changes for you, the owner.

While getting familiar with the codebase, a few areas looked like possible improvement candidates:

• Adding automated tests (starting with small, behavior-preserving coverage)
• Opt-in authentication for network control paths, with backward compatibility for existing users
• Safer JSON handling (tests first, then incremental refactors)
• Network/input parser hardening (e.g., safer bounds-checked parsing of MQTT/WS/URI input)
• Thread-safety around shared state access

I’m happy to start small with narrow, review-friendly PRs. I'm not proposing big refactors.

If you’re open to contributions, could you share:

1. Which areas you’d most like help with right now (See list above)
2. Which kinds of changes you would prefer to avoid
3. Whether you want issue-first discussion before PRs
4. Whether this project prefers no new deps, or if vendored lightweight libs are acceptable (ex: for JSON building)

Thanks for building and maintaining this project!
Dan

[sfeakes]

Thanks for the support and questions, and the PR's you have submitted today.

The biggest trap people fall into with PR's is the Jandy Panels can be different between version on particular protocols, so when someone might change how AqualinkD interoperates with the panel it may well work for their specific Panel but will break other versions. This is mainly the case for PDA & OneTouch protocols, Allbutton, RSserial protocols are very solid between versions, and iAqualink and AqualinkTouch somewhere in-between. So anything that's touching a Jandy protocol I need to do a ton of testing before accepting the PR.

On the Opt-in authentication for network control paths, I'm not sure where you are going, that can mean a few different things, So be interested to understand that better.

The JSON handling within AqualinkD could do with some better thought and structure, it's evolved over time and was / is designed for speed and no external library dependancies.
I also know there are some potential issues with thread safety, and network input handling.

I prefer not to depend on external libraries as AqualinkD is designed to be lightweight and fast, usually external libraries are highly generic and support many features that AqualinkD doesn't require, making them fat / bloated for this use case but great for other projects. So any dependancies should be discussed first. I'm not against them, but I'm very reluctant to bloat. That's the reasons I picked mongoose as the network library.

One thing I would stay away from is adding functionality that AqualinkD was never intended to do / be or support for highly specific integrations, but you didn't list any of that.

In general AqualinkD is highly stable, lightweight, easy to compile, and goes through a ton of testing before each release, I don't want anything to distracts from that stability. People have had AqualinkD running for years and not upgraded because it sits their any works without any maintenance, they kind-a forget about it, this is a good thing.

[tehranian]

Thanks for the thoughtful response, Shaun — stability and "it just works" is clearly a priority, and that comes through in the design choices.

I've been digging into the codebase and put together some writeups on the areas you mentioned.

1. Test coverage proposal — Probably the least controversial of the three. This adds a make test target and ~75 unit tests covering the utility functions, checksum verification, JSON parsing, and string helpers. Importantly, it doesn't touch any existing source files — just adds a test/ directory with Unity (3-file MIT-licensed test framework) and test files. The production build is completely unaffected.

2. JSON migration proposal — You mentioned the JSON handling could use better structure. This evaluates two approaches (Mongoose's built-in JSON API vs. cJSON) and recommends cJSON. The main win is eliminating the structural bugs that come from hand-rolled sprintf chains — it's impossible to produce malformed JSON with a DOM builder. I'd want the test infrastructure in place first so the migration is verifiable at every step.

3. Auth proposal — This is the "opt-in authentication for network control paths" I mentioned. It's a phased approach: off (current behavior, default) → audit (log who's doing what) → write (protect mutations) → all (full lockdown). Uses Mongoose's built-in credential parsing and SHA-256, so no new dependencies. I'm still waiting on my own hardware before I'd want to push code here — I'd rather test it against a real setup.

On the testing front — you mentioned doing a ton of testing before accepting protocol PRs, and I'm curious what that looks like today. Do you have the various hardware types, or do you rely on community members to validate? It seems like an area where automation or simulation could help — I noticed the existing dummy_reader and dummy_device utilities are already 80% of what you'd need for capture-based regression testing, if users contributed packet recordings from their specific panel versions.

[sfeakes]

@tehranian Thanks for the this. Few comments after reading over it all.

1. Test coverarge

That's fine, just make sure they are valid test cases.

2. JSON

It missed some JSON parsing areas (specifically the scheduler).

I agree the Mongoose (MG) JSON approach is not the direction to go, which is why I haven't used it. If you look at how AqualinkD is currently implemented, it would be easy to add the missing overflow tests, which was always the original intent.

Moving over to cJSON is a good suggestion. However, I am very concerned about memory fragmentation and speed overhead. If you want to test this approach, please copy the cJSON.c and cJSON.h source files directly into this repo—do not make it an external library dependency.

Thinking about cJSON overhead makes me wonder: rather than creating the larger JSON structures (like devices & status) on the fly for every request, it would be much more beneficial to simply keep an up-to-date, pre-rendered copy of them in memory and only rebuild (or preferably modify them) when a state actually changes.

Regarding the proposal to use quotes around integers: I disagree. Simply change the UI to accept standard JSON integers. The quotes were a historical implementation for backward compatibility with older embedded browser JSON parsers, which is no longer relevant.

3. Auth

I think this proposal misses the mark on the project's core philosophy, though I'm open to discussion.

AqualinkD is not designed to be public-facing or operate in a zero-trust environment. It is designed for the home network, relying on a "castle wall" security model. If a bad actor already has access to your private LAN, you have much larger problems than whether they can turn on your pool pump.

Adding application-level security (passwords, session management) to a lightweight daemon is generally a lost cause and often introduces more vulnerabilities than it solves. There are dedicated systems explicitly designed for securing applications (Nginx, Apache, Traefik, Home Assistant). If a user wants public/secured access, the correct architectural approach is to put AqualinkD behind a reverse proxy. There is already a big disclaimer on the main page regarding this.

mTLS is already supported and can be set through the AqualinkD configuration. This is the ultimate / gold standard way to lock everything down for machine-to-machine communication. There is no need for users, passwords, API tokens, or session management when mTLS is available.

Any network auth without HTTPS is essentially useless. But forcing HTTPS locally means dealing with self-signed certs, which are a massive headache for the average user on modern browsers.

What may be beneficial here is to provide some documentation & sample configurations for Nginx / Apache etc.

---

On the testing against protocols, there are a few ways this is implemented.

• Specific problem against HW that I can't replicate is usually tested with the user who raised an issue.
• Jandy does have a very set of old simulators (you can find it on the net), some of the applications talk directly to serial port and use RS485. I have that running in an old windows 7 VM that's sometimes used for testing.
• When I find information about new devices, I will usually write a "simulator" application that's used in testing. The dummy_device file you see is simply a stub to set that up. I do not publish that code.
• I have old panels / control boards / interfaces that people have donated over the years, and test against those. You don't need a complete power center / device to get the control boards up and running, just the circuit board and a few resistors to fake output / readings etc.