Repository navigation
RTOP-376: Rewrite the RTC block, cite IEC 61131-3 on the example blocks, and add the CLA - #254
Conversation
…-3 on the example blocks The RTC block was carried over from MatIEC's LGPL library, which Autonomy does not own. It is reimplemented from its documented behaviour, unchanged for users: the runtime clock while IN is FALSE, PDT latched on the rising edge, PDT plus elapsed time while IN stays TRUE. New tests pin that behaviour. PID, INTEGRAL, DERIVATIVE, RAMP and HYSTERESIS are the IEC 61131-3 example blocks, so their headers now cite the standard instead of MatIEC. A stale note claiming RTC calls CURRENT_DT() is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRWeC5FUhe5ZwEEbLUGpZ6
…l request check Contributions are accepted under CLA.md, so they can be offered under the licences of the OpenPLC ecosystem. Submitting a pull request means agreeing to it, the pull request template carries a checkbox, and a CLA check fails until outside contributors tick it. The README points to the runtime licensing page. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRWeC5FUhe5ZwEEbLUGpZ6
Copyright lines and the package author name Autonomy alone. The IEC example blocks cite IEC 61131-3, 2nd Ed. without a draft date. The CLA is accepted by ticking its pull request checkbox, and the check runs as a job named CLA. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRWeC5FUhe5ZwEEbLUGpZ6
Gustavohsdp
left a comment
There was a problem hiding this comment.
Review summary
The RTC rewrite is behaviourally equivalent to the previous block (edge detection, PDT latch, CDT = PDT + elapsed, Q := IN), the CLA workflow reads the PR body through env (no script injection), and the five IEC example blocks match the MatIEC originals apart from formatting and the header.
Validation run by the reviewer (Linux, node:22-bookworm + g++): npx vitest run → 104 files, 3474 passed / 7 skipped; npm run typecheck clean; npm run lint 0 errors. On macOS the g++-based tests fail at link time (tapi error: malformed file), which is a local toolchain problem and also hits suites this PR does not touch.
Findings (details inline):
| Severity | Where | Problem |
|---|---|---|
| required | README.md:274 |
The OpenPLC Runtime licensing link returns 404: LICENSING.md only exists in openplc-runtime#225, which is still open and targets development, not main. |
| required | tests/library/additional-fb-rtc.test.ts:25 |
The IN = FALSE case only checks a delta, so the epoch base of CDT is not pinned. A mutant that uses DT#2000-01-01-00:00:00 + TIME() passes all six cases. |
| nit | libs/sources/additional-function-blocks/rtc.st:2 |
Only rtc.st gets an SPDX/copyright header; the other five .st files in the same library have none. The holder also reads "Autonomy", while CLA.md names "Autonomy Logic Inc.". Is that intentional? |
| nit | libs/sources/additional-function-blocks/rtc.st:9 |
The inline descriptions of IN, PDT, Q and CDT were dropped, and they were the only per-variable documentation for this block. |
| nit | .github/workflows/cla.yml:22 |
author_association can report CONTRIBUTOR for members whose org membership is private, so they would have to tick the box too. Harmless, but worth knowing. |
Note (outside this PR): ci.yml only runs on pull_request to main, so PRs into development, this one included, get only the CLA check and no lint, typecheck or tests in CI.
… staff Licence identifiers name GPLv3 only, matching the CLA's GPLv3 commitment. The CLA is versioned and staff tick it too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRWeC5FUhe5ZwEEbLUGpZ6
RTC reads CURRENT_DT() once on its first scan and counts from it with TIME(), so it starts at the system date (1970 plus uptime on targets without one). While SET is TRUE, CDT equals PDT; once SET is FALSE, CDT keeps counting from the last value set. BREAKING CHANGE: the IN input is renamed SET. While SET is FALSE, CDT no longer falls back to the runtime clock. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRWeC5FUhe5ZwEEbLUGpZ6
|
Review follow-up:
|
libs/sources/additional-function-blocks/rtc.st). Its predecessor was carried over from MatIEC's LGPLv3 library, which Autonomy does not own. Breaking behaviour change, decided in review: RTC is now a set-and-run clock. On its first scan it readsCURRENT_DT()once and counts from it withTIME(), so it starts at the system date (1970 plus uptime on targets without one, such as AVR). The IN input is renamed SET: while SET is TRUE, CDT equals PDT; when SET is FALSE, CDT keeps counting from the last value set. Q copies SET. Projects that wire IN must rename it to SET. The block's hover documentation describes this.Copyright (c) <year> Autonomy®and every licence identifier readsGPL-3.0-only(with the runtime exception for the library).CLA.md,CONTRIBUTING.md, the PR template checkbox and aCLAcheck. Everyone ticks it, staff included; only bots pass without it.CLA.mdis version 1.0. The README points to the runtime licensing page.Ticket: RTOP-376 — https://autonomylogic.atlassian.net/browse/RTOP-376 (small change, no Requirements Gathering; no Cybersecurity Risk Assessment: library code, text and repository configuration only).
Testing
tests/library/additional-fb-rtc.test.ts(six ST cases through g++): never set (starts within 5 s ofCURRENT_DT()and counts), SET TRUE, SET held, SET released, PDT ignored while SET is FALSE, setting again. Deliberately broken blocks (no set time; a 1970 start) fail the matching cases.npx vitest run tests/library: 489 passed;npm run typecheckclean.CURRENT_DT()from the new RTC, AVR included.Contributor License Agreement
🤖 Generated with Claude Code
https://claude.ai/code/session_01MRWeC5FUhe5ZwEEbLUGpZ6