nutdrv_qx: add OMRON BN150T support - #3554
Open
junkurihara wants to merge 12 commits into
Open
Conversation
…orkupstools#3112] Same transport as "ippon", except the control transfer carries a 16-byte HID Output report, the size declared by the BN150T's report descriptor. The send loop advances by the report size and rejects a short transfer, and the reply length and both debug prints are bounded by the number of bytes actually read. Verified on 0590:00b7 with protocol=q1. Other OMRON models untested. Signed-off-by: Jun Kurihara <junkurihara@users.noreply.github.com>
…" subdriver [networkupstools#3112] nut-usbinfo.pl generates the udev, hotplug and devd rules from the driver device tables, so without a qx_usb_id[] row a packaged install grants the device no permissions at all. The row needs a matching omron_subdriver(): qx_is_usb_device_supported() only reports SUPPORTED once a handler has assigned subdriver_command. Only the tested model is listed; the other OMRON IDs shipped by the vendor driver are untested. Signed-off-by: Jun Kurihara <junkurihara@users.noreply.github.com>
…etworkupstools#3112] The BN150T reports the third Q1 status bit as 1 continuously while on mains, idle and unloaded, which "q1" turns into BYPASS - in NUT convention a state where protection has been bypassed. OMRON's own driver does not evaluate that bit at all: the block deciding between TRIM, BYPASS and BOOST is commented out in its entirety, identically in the 1.00 and 1.02 vendor releases, while every other status bit is handled. The row is dropped here rather than remapped, since what the bit reports on this hardware is not known. Instant commands the vendor driver does not implement are not registered: test.battery.start, .deep, .quick and .stop have no T or TL command anywhere in either vendor tree, the vendor's instant command set has no whole-load on/off behind load.on and load.off, and beeper.toggle has no bare Q. Registering an instant command tells clients the device can perform it. The shutdown commands use OMRON's byte strings. The vendor sends "S<n>" and "Sf<n>" with the delay in tenths of a minute below one minute and in whole minutes above, and never appends the Megatec "R<mmmm>" return-delay field; blazer_process_command() appends it to both as soon as ups.delay.start is non-zero, and spells stay-off as "S<n>R0000" where OMRON has a distinct "Sf". shutdown.return and shutdown.stayoff are also two transactions rather than one: the auto-restart flag is set with "An" or "Af" first, as both the vendor driver and docs/nut-names.txt require, and a failure there abandons the command instead of powering the unit down with its restart behaviour unknown. A command the UPS accepts is acknowledged with "OK", not the "ACK" that the Megatec-derived subdrivers expect. The claim is blazer_claim_light() gated on OMRON's USB vendor ID. Without the gate this subdriver, which checks no more than "q1" does, would claim every device the generic q1 fallback exists to serve. A serial-attached unit therefore needs protocol=omron set explicitly. The USB transport handler added earlier is renamed omron_usb_subdriver(), because the subdriver_t structure takes the plain name; same treatment as fabula_hunnox_subdriver(). Verified on 0590:00b7: polling reports OL without the spurious BYPASS. No instant command has been sent to the hardware, so every command string here rests on the vendor source alone. Other OMRON models untested. Signed-off-by: Jun Kurihara <junkurihara@users.noreply.github.com>
…s#3112] Adds the OMRON BN150T to the compatibility list at support level 4, the protocol to the nutdrv_qx man page, the subdriver source files to the developer guide's list, and the two words the spell checker needs. The man page states what was actually exercised: the polling path on one model over USB. None of the shutdown commands has been sent to real hardware, so the section says so rather than implying coverage. ondelay is accepted for this protocol but does not reach the device: OMRON's shutdown commands carry no return-delay field. Signed-off-by: Jun Kurihara <junkurihara@users.noreply.github.com>
Add BN150T extension queries, OMRON-specific status handling, and shutdown command encoding and acknowledgement handling. Prefer the vendor-ID-gated OMRON protocol during USB autodetection, and document the tested scope and known limitations. Signed-off-by: Jun Kurihara <junkurihara@users.noreply.github.com>
…#3112] Identify the OMRON-authored driver versions as 1.00 from the Synology DSM 7.3-86009 GPL sources and 1.02 from the QNAP QTS 5.2.3 GPL sources. Replace ambiguous references to vendor releases consistently across the manual, NEWS, driver list and source comments. Signed-off-by: Jun Kurihara <junkurihara@users.noreply.github.com>
Signed-off-by: Jun Kurihara <junkurihara@users.noreply.github.com>
Improve the wording of the OMRON NEWS entry, manual section and source comments. Clarify the tested scope, protocol behavior and source provenance while keeping the private report details out of the public documentation. No driver logic is changed. Signed-off-by: Jun Kurihara <junkurihara@users.noreply.github.com>
…ools#3112] Add DSM, OMRON's, QNAP and Synology to the spelling dictionary for terms introduced by the OMRON documentation. Signed-off-by: Jun Kurihara <junkurihara@users.noreply.github.com>
|
A ZIP file with standard source tarball and another tarball with pre-built docs for commit c9be88e is temporarily available: NUT-tarballs-PR-3554.zip. |
|
✅ Build nut 2.8.5.5018-master completed (commit 60e9165a5b by @junkurihara)
|
There was a problem hiding this comment.
Pull request overview
This PR extends the nutdrv_qx driver family with OMRON BN150T (USB 0590:00b7) support by adding a dedicated USB transport and a protocol subdriver derived from the Q1 family, plus the related documentation and metadata updates.
Changes:
- Added an
omronUSB transport andomronprotocol subdriver with OMRON-specific parsing, extensions, and shutdown command handling. - Registered the BN150T USB ID and enabled OMRON-gated autodetection/selection.
- Updated documentation (manual, subdriver list, NEWS), spell-check dictionary, and driver list metadata; bumped driver version to 0.54.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| NEWS.adoc | Announces the new OMRON BN150T support and summarizes behavior/limitations. |
| drivers/nutdrv_qx.c | Bumps driver version; registers OMRON protocol/USB transport and USB VID:PID. |
| drivers/nutdrv_qx_omron.h | Declares the new OMRON protocol subdriver interface. |
| drivers/nutdrv_qx_omron.c | Implements the OMRON protocol subdriver, extensions, and shutdown commands. |
| drivers/Makefile.am | Ensures the new OMRON subdriver sources/headers ship and build. |
| docs/nutdrv_qx-subdrivers.txt | Adds the OMRON subdriver to the documented list. |
| docs/nut.dict | Adds OMRON-related words to the spell-check dictionary. |
| docs/man/nutdrv_qx.txt | Documents the new omron protocol and omron USB subdriver options and behavior. |
| data/driver.list.in | Adds a BN150T entry with a recommended ups.conf configuration line. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Describe the subdriver option as selecting a USB communication subdriver so it also covers the OMRON HID transport. Document that OMRON's non-standard SET_REPORT value 0x0002 is intentional, matches both vendor driver releases, and was verified on the BN150T. This changes only the literal's presentation, not its value or behavior. Signed-off-by: Jun Kurihara <junkurihara@users.noreply.github.com>
|
✅ Build nut 2.8.5.5019-master completed (commit c15c0ac91c by @junkurihara)
|
Cast memchr() results explicitly to char * so C++ compatibility checks do not reject the implicit conversion from void *. Signed-off-by: Jun Kurihara <junkurihara@users.noreply.github.com>
|
❌ Build nut 2.8.5.5023-master failed (commit f3ca3ab2c1 by @junkurihara) |
Author
|
AppVeyor build 5023 appears to have hit the 60-minute job timeout while packaging the Windows artifacts. The build and install stages completed successfully, and the log ended during the final 7z command without a compiler or test error. Could someone please rerun it? |
Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
jimklimov
added
enhancement
USB
Qx protocol driver
Member
|
Updated the spellcheck dictionary that some hosts complained about, this should re-build the Windows job as well. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
✅ Build nut 2.8.5.5025-master completed (commit 2d431f7d34 by @jimklimov)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Key Changes
omronUSB transport for OMRON BN150T (0590:00b7). It uses the device's 16-byte HID Output report size, rejects short writes, and bounds reply parsing and debug output by the number of bytes actually received.omronprotocol subdriver based on the Q1 family, with OMRON-specific status handling, strict validation, extension queries, and automatic USB selection guarded by OMRON's vendor ID.shutdown.stop,shutdown.return, andshutdown.stayoff, including the requiredAn/Afauto-restart prologue andOKacknowledgement handling.nutdrv_qxdriver version to 0.54.Testing
make -j2 -C drivers -W nutdrv_qx_omron.c nutdrv_qx: exit 0, no warnings.nutdrv_qx_omron.cwith-DTESTING -fsyntax-onlyand the project warning flags: exit 0, no warnings.git diff --check 26177060b: exit 0, no output.0590:00b7. All three paths selectedOmron 0.01; autodetection sent no other protocol probes.shutdown.stop,shutdown.return, andshutdown.stayoff, including 120-second schedule/cancel checks and actual 12-second shutdowns. The observed command sequences and acknowledgements matchedC,AnthenS.2/S02, andAfthenSf.2/Sf02as expected.shutdown.returnwhile on battery and automatic output recovery after utility restoration. Verified thatshutdown.stayoffheld the output off with utility present until manual front-panel recovery.shutdown.default,upsdrvctl shutdown, the driver-kpath, ordriver.killpowerbecause of the separately reported shared-core failure-masking issue (drivers: shutdown.default can report success afterupsdrv_shutdown()fails #3553).make distcheck: exit 0. This covered the distribution archive, an all-features configured build, checks, and normal andDESTDIRinstall/uninstall passes, including the new subdriver files.tools/nut-usbinfo.pl. The BN150T appears in the generated udev, devd, hotplug and nut-scanner data. The trackedscripts/upower/95-upower-hid.hwdbis unchanged because that generator emits entries there only forusbhid-upsandapc_modbusdevices.make spellcheckwithaspelland its English dictionary: exit 0, with no findings after addingOMRON's,DSM,QNAP, andSynologytodocs/nut.dict.Related Tasks
upsdrv_shutdown()fails #3553 tracks the pre-existing failure-reporting limitation in sharedshutdown.defaultand driver-khandling encountered while reviewing shutdown behavior. This PR intentionally does not modifydrivers/main.c.Other
ups.start.auto. Automatic recovery after a non-cancelledAnshutdown was physically verified, but preservation of the finalAnsetting acrossCremains an inference rather than a device readback.Below is a checklist from the PR template.
General points
Described the changes in the PR submission or a separate issue, e.g. known published or discovered protocols, applicable hardware (expected compatible and actually tested/developed against), limitations, etc.
There may be multiple commits in the PR, aligned and commented with a functional change. Notably, coding style changes better belong in a separate PR, but certainly in a dedicated commit to simplify reviews of "real" changes in the other commits. Similarly for typo fixes in comments or text documents.
Use of coding helper tools and AI should be disclosed in the commit or PR comments (it is interesting to know which ones do a decent job). As with other contributions, a human is responsible and thanked for the quality and content of the change, and is presumed to have the right to post that code to be published further under the project's license terms.
Especially with involvement of AI, including modern IDE coding aid, please be sure to revise that proposed code and documentation changes follow NUT code style guide -- this helps portability across the decades worth of supported systems. Notably, avoid Unicode characters where ASCII text is expected (C sources and headers, manual pages and other
acsiidocinputs). Particularly AI is keen on addingmdashcharacters instead of plain ASCII double-dash (which renders into the long dash where applicable).Please star NUT on GitHub, this helps with sponsorships! ;)
Frequent "underwater rocks" for driver addition/update PRs
Revised existing driver families and added a sub-driver if applicable (
nutdrv_qx,usbhid-ups...) or added a brand new driver in the other case.Did not extend obsoleted drivers with new hardware support features (notably
blazerand other single-device family drivers for Qx protocols, except the newnutdrv_qxwhich should cover them all).For updated existing device drivers, bumped the
DRIVER_VERSIONmacro or its equivalent.For USB devices (HID or not), revised that the driver uses unique VID/PID combinations, or raised discussions when this is not the case (several vendors do use same interface chips for unrelated protocols).
For new USB devices, built and committed the changes for the
scripts/upower/95-upower-hid.hwdbfile-> N/A for this
nutdrv_qxdevice:tools/nut-usbinfo.plwas run and produced no change to that file because its UPower output includes onlyusbhid-upsandapc_modbusdevices.Proposed NUT data mapping is aligned with existing
docs/nut-names.txtfile. If the device exposes useful data points not listed in the file, theexperimental.*namespace can be used as documented there, and discussion should be raised on the NUT Developers mailing list to standardize the new concept.Updated
data/driver.list.inif applicable (new tested device info)Frequent "underwater rocks" for general C code PRs
Did not "blindly assume" default integer type sizes and value ranges, structure layout and alignment in memory, endianness (layout of bytes and bits in memory for multi-byte numeric types), or use of generic
intwhere language or libraries dictate the use ofsize_t(orssize_tsometimes).Progress and errors are handled with
upsdebugx(),upslogx(),fatalx()and related methods, not with directprintf()orexit(). Similarly, NUT helpers are used for error-checked memory allocation and string operations (except where customized error handling is needed, such as unlocking device ports, etc.)Coding style (including whitespace for indentations) follows precedent in the code of the file, and examples/guide in
docs/developers.txtfile.For newly added files, the
Makefile.amrecipes were updated and themake distchecktarget passes.General documentation updates
Added a bullet point into
NEWS.adoc, possibly alsoUPGRADING.adocif there is something packagers or custom-build users should take into account (new driver categories, configuration options, dependencies...)->
UPGRADING.adocis not applicable because this adds no dependency, incompatible configuration change, or packaging migration.Updated
docs/acknowledgements.txt(for vendor-backed device support)-> N/A: the implementation uses published GPL sources, but there was no vendor-backed contribution or claim of current official support.
Added or updated manual page information in
docs/man/*.txtfiles and corresponding recipe lists indocs/man/Makefile.amfor new pages-> The existing
docs/man/nutdrv_qx.txtpage was updated; no new manual page or recipe was added.Passed
make spellcheck, updated spell-checking dictionary in thedocs/nut.dictfile if needed (did not remove any words -- themakerule printout in case of changes suggests how to maintain it).Additional work may be needed after posting this PR
Propose a PR for NUT DDL with detailed device data dumps from tests against real hardware (the more models, the better).
Address NUT CI farm build failures for the PR: testing on numerous platforms and toolkits can expose issues not seen on just one system.
Revise suggestions from LGTM.COM analysis about "new issues" with the changed codebase.