New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add nutdrv_qx driver for Ablerex model PR#2 #1135
Add nutdrv_qx driver for Ablerex model PR#2 #1135
Conversation
Thank you, it is much more manageable now, and great thanks for the patience and persistence to get this done right :) Seems you On non-coding side, it would be beneficial to also update the documentation similar to those seen in https://github.com/networkupstools/nut/pull/979/files -- at least:
Regarding VID/PID, should there be a change to USB_DEVICE table like at https://github.com/networkupstools/nut/pull/979/files#diff-12d702aa91ce0aee7706c1aa0275279fc8059acc58e69088116528e1b20509b7R446-R447 below?
For maintainer context, I believe this PR follows up from earlier contribution attempts in PRs #1123 and #979. |
On codebase side, seems there are a few complaints from CI checks, e.g.:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the very least, there are a few codebase fixes needed to pass CI builds cleanly.
Also, some documentation changes are recommended above.
Thank you for your help with this pull request.Yes. this request was updated PR-979 and created a sub driver for Ablerex.
We will spend time and update these documents soon.
|
Hi! Sir, If we added the USB_DEVICDE table as below at nutdrv_qx.c |
Hello. Given that the line in question at https://github.com/networkupstools/nut/blob/master/drivers/nutdrv_qx.c#L1460 has a comment that it is, for example, to handle "Ablerex 625L USB", I supposed that the new vendor-backed subdriver for the same/related device would be the replacement for the original one. At worst, users can select the subdriver explicitly in their Otherwise, the two commonly ADDON NOTE: I think in some drivers I've also seen hooks for more thorough device interaction to make sure it is a supported one by the specific subdriver, or move on to a better match, but can't quickly point a finger so maybe it was not done in On pedantic side, tell the HW people that this is exactly the situation why the USB Forum organization is there, to license vendor (and product) IDs, same as standard authorities for DNS names, IP addresses and ports, phone numbers or whatever, and why kidnapping bogus numbers like "0x0000" and "0xffff" (or any other "owned" by someone) is a cheap but bad idea interops-wise ;) |
Hi Sir, |
Regarding |
As for
Are you sure at least those would be well supported by the new subdriver? (Maybe yes, if someone is same OEM for all the brands) If not, I would ask you to explore adding a line in that table in
If your devices expose some other "iVendor" names in USB discovery, you can add more lines to match those. As seen in https://github.com/networkupstools/nut/blob/master/drivers/nutdrv_qx.c#L1497 the comparison is done with |
Hi sir, ;------------------------------------------------------------------------------ IPAR/IPAR__Mini_Energy_ME_800__megatec_usb__2.6.0__01.dev:ups.productid: 0000 Liebert/Liebert__PSA1500__blazer_usb__2.6.0__01.dev:ups.productid: 0000 UPSonic/UPSonic__IRT1000__blazer_usb__2.7.1__01.dev:ups.productid: 0000 I believe the internal firmware of UPS will be compatible with the currently existing(krauler_subdriver at 0xffff/0x0000) and even if change to new driver( ablerex_subdriver) will be compatible, too. |
In that case, I think this vid:pid can be reassigned to new subdriver by default; if some users' devices won't be handled well, their config files can request another. I'll mention that in release notes. |
@jimklimov According to our explanation for this PR, we need your support, so please kindly give us some comments. |
@jimklimov ;----------------------------------------------------------------------------------------------------------------- Is that right, please? |
Sorry for the delay - Yes, in this case that seems to be the acceptable approach. |
Note: there are several CI warnings, regarding libusb method argument types (they varied for different library generations) and some unused variables and methods:
Some of this is very similar to something rectified with |
We update the PR to modify the ci warning, replace Ablerex_subdriver and documents. Please kindly review. |
Whitespace fix
Weird... Got this issue on some of the CI platforms:
...but looking at it closer, I'd expect it to be all over the place. It highlighted a naming deficiency in this driver, where One recent example that had both sorts of the name defined was Updating PR to introduce the |
Hooray! Looks good to me and passed the CI build matrix now. Would you please be able to test the currently proposed codebase (refreshed from this PR source branch) against your devices, to make sure the fixes did not break actual hardware support? |
The below link drawing is for the new driver test result. Please Kindly check for us. |
Is the table of supported devices in #979 still valid for this new PR?
For the driver.list.in updates, I suppose it fits in the current PR well;
the support level would be 4 or 5 (IIRC for vendor supported contribution).
It would be great to also post a PR with "device-data dumps" (specially
formatted text files) to NUT-DDL repository, with samples of various models.
Naming and data conventions are detailed in the docs and rendered starting at https://networkupstools.org/networkupstools-master.github.io/ddl/index.html#_file_naming_convention section.
Large-change merges will be done in a few days, the NUT CI farm hosting
should be migrated this week so just in case I suspended merge activities
to not lose build-quality history on the main branches.
Jim
|
I'm sorry for too late reply to your message. this PR(#1135) will support PR(#979). We update and post the UPS data of device-data dumps to https://github.com/networkupstools/nut-ddl. |
Updated with current improvements from master branch (including libusb-1.0 support and some bug fixes that remained for "fightwarn effort", so now default warnings level is higher). If the next builds pass CI farm okay, hoping to merge this soon. Great thanks for the effort (large part of that sadly and unavoidably being the rituals needed to make this work well for everyone in the community). |
CI says (and review confirms) this PR needs some more changes to adjust to the upstream-master codebase evolution. In particular due to the libusb-1.0 support merge, the |
Fix nut_usb_strerror(ret) for new libusb-1.0/0.1 support in master branch
… and formatting changes for libusb-1.0 support
Congratulations, this PR finally looks good to CI - no warnings, no conflicts (one agent went AWOL in last iteration, not your fault). |
Thanks for your guidance and help to make this PR can be merged smoothly. |
A short statement goes to `data/drivers.list.in` - that such device is
handled by such driver.
Helpful device data dumps go as PRs to nut-ddl repository.
The website currently is refreshed manually, so there's a lag for that data
to be exposed beyond source repos :(
…On Mon, Jan 24, 2022, 06:37 Ablerexsoftware ***@***.***> wrote:
Thanks for your guidance and help to make this PR can be merged smoothly.
In addition, how should we do if we want to add a new model in the
"Hardware compatibility list" of Network UPS Tools?
—
Reply to this email directly, view it on GitHub
<#1135 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMPTFFZ55FKCVH42ZL45NTUXTQS3ANCNFSM5GGFVBNQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
Thank you for kindly providing the information to us. |
The site is updated by NUT team, occasionally. Currently the site reflects
latest release that people see packaged (which is always something old), so
there was some rearchitecting underway to publish historic data separately
and current as the main site.
…On Fri, Jan 28, 2022, 04:50 Ablerexsoftware ***@***.***> wrote:
Thank you for kindly providing the information to us.
We have pulled the ddl files to nut-ddl repository for this PR before.
Does the website need to be refreshed by the Nut team or do we still pull
the repository to refresh? Please kindly tell me which repository, if we
need to pull the PR to refresh the website.
—
Reply to this email directly, view it on GitHub
<#1135 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMPTFBAAB7UNS34ANS7XOTUYIHAJANCNFSM5GGFVBNQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
Website updates progressed last weekend, so the main https://networkupstools.org/ site should track master branches of NUT and DDL more closely (and the older "historic" releases published in sub-sites for reference of their users stuck with the older codebase). The compatibility information from git should be up there now. |
Thank you for updating the information to us. |
Hello, packaging and recipes for that is something every OS distribution
does on their own. While I would be in favor of NUT providing "reference"
recipes so people have less differences and patches per OS, everyone is
going to be unique anyway.
Most of them use latest releases of projects they package, however, and NUT
is tying up some loose ends to issue a 2.7.5 which would include the
changes here.
…On Mon, Feb 14, 2022, 02:43 Ablerexsoftware ***@***.***> wrote:
Thank you for updating the information to us.
May I consult about the Nut package of Linux?
Does the nut package was updated to the latest(with the same master
branches of git)? if we download and install it directly from Linux
system(e.g. Ubuntu..)
—
Reply to this email directly, view it on GitHub
<#1135 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMPTFCXR3EPIQNVR5R7ILDU3BM3BANCNFSM5GGFVBNQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
I am sorry for that.
We set a new branch and repush it for nutdrv_qx driver for Ablerex model, please review