Hi, Well.. it's my understanding that everything < 1 should be some sort of error (0 is a timeout).. in my opinion the point here is that the driver is expecting no reply at all, so, yes it is expecting an error.. I think this was done to cover a wide array of devices using a protocol similar to the megatec one http://www.networkupstools.org/ups-protocols/megatec.html However since some of them seemed to reply 'ACK' in case of success, that exception was added (as you suggest, I think we should add it also in the check done at line 421, maybe adding also '(ACK' in both of them, as it should not be of any harm to the existing users of the driver.).
My suggestion for the voltronic driver was just based on the use of (ACK/(NAK and the ProductIT/VendorID as they seem to be the same used by that manufacturer - as well as many others, I think - so I thought that maybe your UPS was one of the many rebrands of that manufacturer (these UPSes still support the 'old' Q1 idiom used by the blazer drivers but have the (ACK/(NAK peculiarity you are experiencing too). If that was the case, using that driver you'd get a better control over what's going on. Nonetheless, I think we may have a better chance with a custom subdriver for the blzr driver: https://github.com/zykh/nut/tree/blzr I just added one (i.e. protocol 'mecer') that should cover your situation (i.e. an idiom similar to the one used by the megatec protocol but an expected reply of '(ACK' in case of success, '(NAK' in case of errors. I didn't mention it the last time since it hasn't been checked yet by the other developers. Cheers /dan 2013/10/25 Johan Meiring <[email protected]>: > Hi, > > They say one should never argue with people that know more than you, so > please bear with me. > > On 2013/10/24 09:47 PM, [email protected] wrote: >> The driver is expecting either 'ACK' or no reply at all in case of >> success and the command itself echoed back in case of errors. >> On the other hand, your UPS replies '(ACK' in case of success and >> '(NAK' on failure. > > This I agree with, but... > >> >> So.. when the driver first tries to stop pending shutdowns and you get >> the 'operation not permitted' error, the driver interprets it as a >> success. > > Looking at the debug log, this is actually a failure. > The 'operation not permitted' is a comms failure with the UPS. > > Later on in the debug log you can see where it tries to cancel the pending > shutdown, a "(NAK" is sent, and not a 'operation not permitted' error. > > In any case, this is actually considered to be a success by the code. > This is not right. > > Code as follows: > 418 /* > 419 * If a command is invalid, it will be echoed back > 420 */ > 421 if (blazer_command(buf, buf, sizeof(buf)) > 0) { > 422 upslogx(LOG_ERR, "instcmd: command [%s] failed", cmdname); > 423 return STAT_INSTCMD_FAILED; > 424 } > 425 > 426 upslogx(LOG_INFO, "instcmd: command [%s] handled", cmdname); > 427 return STAT_INSTCMD_HANDLED; > > As far as I understand the code line 421 tries to test for a 'comms failure' > (e.g. driver disconnection) or any return string. I also suspect that comms > failures are negative numbers. > Should the test not be "!= 0". > > What is clear though is that the code does not try to check the response. > If there was not a comms failure or a return string, it is considered handled. > > Should it not also test for an "ACK" or "(ACK"? > >> When it tries to shutdown.return and you get '(ACK', the driver >> interprets it as a failure.. > > Again looking at the code. > 462 /* > 463 * If a command is invalid, it will be echoed back. > 464 * As an exception, Best UPS units will report "ACK" in case of success! > 465 */ > 466 if (blazer_command(buf, buf, sizeof(buf)) > 0) { > 467 if (strncmp(buf, "ACK", 3)) { > 468 upslogx(LOG_ERR, "instcmd: command [%s] failed", cmdname); > 469 return STAT_INSTCMD_FAILED; > 470 } > 471 } > 472 > 473 upslogx(LOG_INFO, "instcmd: command [%s] handled", cmdname); > 474 return STAT_INSTCMD_HANDLED; > > It seems from the code that the test on line 466 tests for comms failures > (again e.g. driver disconnection) or a return string. On line 467 if there > was a failure, it checks for 'ACK' and this will override the failure. > > Similarty the test on line 466 should probably be "!=0". >> >> You could try and change line 467 of blazer.c from this: >> >> if (strncmp(buf, "ACK", 3)) { >> >> to this: >> >> if (strncmp(buf, "(ACK", 4)) { >> >> ..this should do the trick. >> > > The following patch might be better? > > --- blazer.c.orig 2013-10-25 10:34:54.000000000 +0200 > +++ blazer.c 2013-10-25 10:49:03.000000000 +0200 > @@ -417,10 +417,13 @@ > > /* > * If a command is invalid, it will be echoed back > + * As an exception, some UPS units will report "ACK" or > "(ACK" in case of success! > */ > - if (blazer_command(buf, buf, sizeof(buf)) > 0) { > - upslogx(LOG_ERR, "instcmd: command [%s] failed", > cmdname); > - return STAT_INSTCMD_FAILED; > + if (blazer_command(buf, buf, sizeof(buf)) != 0) { > + if (strncmp(buf, "ACK", 3) && strncmp(buf, "(ACK", 4) > ) { > + upslogx(LOG_ERR, "instcmd: command [%s] > failed", cmdname); > + return STAT_INSTCMD_FAILED; > + } > } > > upslogx(LOG_INFO, "instcmd: command [%s] handled", cmdname); > @@ -461,10 +464,10 @@ > > /* > * If a command is invalid, it will be echoed back. > - * As an exception, Best UPS units will report "ACK" in case of > success! > + * As an exception, some UPS units will report "ACK" or "(ACK" in > case of success! > */ > if (blazer_command(buf, buf, sizeof(buf)) > 0) { > - if (strncmp(buf, "ACK", 3)) { > + if (strncmp(buf, "ACK", 3) && strncmp(buf, "(ACK", 4) ) { > upslogx(LOG_ERR, "instcmd: command [%s] failed", > cmdname); > return STAT_INSTCMD_FAILED; > } > > Opinions? > Is this the correct place to submit pathes? > >> However, your UPS seems to speak an idiom that's more akin to the one >> covered by the voltronic drivers (i.e. the (ACK/(NAK replies) - >> available in the github repo - so maybe they're worth a try. >> > > I looked at the voltronic.c file. > The protocol seems to be quite different. > The rest of the driver works perfectly. > (Unless voltronic firmware has an older "backwards compatible mode ??) > > The UPS is also a Mecer, sold by a company called Mustek. > and the Blazer Driver seems to mention Mustek in a few places. > > If you still think I must try it, I will gladly do so. > > > Thanks! > > > -- > > > Johan Meiring > Cape PC Services CC > Tel: (021) 883-8271 > Fax: (021) 886-7782 > > -------------------- > Before acting on this email or opening any attachments > you should read Cape PC Service's email disclaimer at: > > http://www.pcservices.co.za/documents/disclaimer.pdf > _______________________________________________ Nut-upsuser mailing list [email protected] http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/nut-upsuser
