nutdrv_qx: add support for SNR-UPS-LID-XXXX#1008
nutdrv_qx: add support for SNR-UPS-LID-XXXX#1008jimklimov merged 9 commits intonetworkupstools:masterfrom
Conversation
…nutdrv_qx Protocol: 'megatec' USB subdriver: 'snr'
jimklimov
left a comment
jimklimov
left a comment
There was a problem hiding this comment.
Looks quite solid to me, thanks! Got one comment for the code to fix, but otherwise looks ok to merge if nobody vetoes this in the next few days :)
Hope @zykh can take a look at this regarding Qx driver/protocol evolution in general.
Just in case, these devices do not need a separate subdriver file for some more complex logic or lookup tables as in e.g. https://github.com/networkupstools/nut/pull/638/files#diff-a94c7d9bf3942c23f12a818a5f0d6f75c8cfb259488ea3c1b93991c0e09b7ca6 ?
Added "snr" and "SNR" to spellchecker dictionary
askazancev
force-pushed
the
nutdrv_qx_snr
branch
from
2bc183c to
432beb0
Compare
Missed a couple of new dictionary words
|
This pull request introduces 1 alert when merging 2fcb47c into 4ac8683 - view on LGTM.com new alerts:
|
jimklimov
added
HCL
need testing
|
One more point came up: the limitation of 102 bytes seemed reminiscent of something - the "hunnox" subdriver introduced with #638 seems to have spell it too at https://github.com/networkupstools/nut/pull/638/files/3a5a758dbbfbb8bdf22a45cdde06fe10d4e0249b#diff-cb890ac7b82cb7a4f861284bcf39cc4f168312b61cb9455eb2755e345c0aa627R922 although the list of commands on top of the (originally fabula) method differs from one introduced for "snr" here. Maybe it does not mean anything, per comments in that codebase seems it originates from a limitation of interaction with USB. In that case however, the buflen passed to the USB routine writing its data into the buffer was not to EXCEED 102 bytes otherwise something froze for that driver developer. Between that data point and yours about buffer needing to be at least 102 bytes, I now wonder if somewhere in spec it just would not say that the buffer must be exactly 102 bytes? :) |
|
Note: nutdrv_qx.c DRIVER_VERSION not bumped here since it was updated a minute ago with merge of PR #638. |
|
This pull request introduces 1 alert when merging 2624bc9 into 169b637 - view on LGTM.com new alerts:
|
| *subdriver =* 'string':: | ||
| Select a serial-over-USB subdriver to use. | ||
| You have a choice between *cypress*, *fabula*, *fuji*, *hunnox*, *ippon*, *krauler*, *phoenix* and *sgs*. | ||
| You have a choice between *cypress*, *fabula*, *fuji*, *hunnox*, *ippon*, *krauler*, *phoenix*, *sgs* and *sgs*. |
There was a problem hiding this comment.
Double sgs. Should be snr and sgs.
There was a problem hiding this comment.
Fixed in master branch after merge, thanks for bringing the attention to this and sorry for the typo.
|
Thanks, sorry, copy-paste error :(
…On Tue, Apr 20, 2021, 14:50 Alexey Kazancev ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In docs/man/nutdrv_qx.txt
<#1008 (comment)>:
> @@ -324,7 +324,7 @@ The argument is a regular expression that must match the bus name where the UPS
*subdriver =* 'string'::
Select a serial-over-USB subdriver to use.
-You have a choice between *cypress*, *fabula*, *fuji*, *hunnox*, *ippon*, *krauler*, *phoenix* and *sgs*.
+You have a choice between *cypress*, *fabula*, *fuji*, *hunnox*, *ippon*, *krauler*, *phoenix*, *sgs* and *sgs*.
Double *sgs*
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#1008 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMPTFCMP345WHBBAQAX54LTJV2CDANCNFSM42UTKULA>
.
|
2 participants
Hello everyone.
In this PR I suggest adding support for UPSes series SNR-UPS-LID-XXXX to nutdrv_qx as a USB device.
An end-user should reference explicitly the
snrsubdriver inups.confsettings because of using the same values of VendorID/ProductID as fabula_subdriver, fuji_subdriver, and krauler_subdriver.