Would like new data appended to file

[Josephny]

I have 1 UPS.

But, the name of that UPS varies from device to device (only because of sloppy/lazy set up on my part).

And, I would indeed like a single (generic) script to use on all devices to log or act on the status of the 1 UPS.

Thank you for the video tutorials. I’ll be watching them next change I get.

[Josephny]

I took the foreach approach and applied to the logging solution and got the following to log low voltage and offline status:

:local upsPrefix "[UPS]:"

/system ups
:foreach item in=[find] do={
    :local iupsname [get $item name]
    :local rawvolt  ([monitor $item once as-value]->"line-voltage")
    :local threshold 115
    :local voltage  ($rawvolt / 100)

      :if ($voltage < $threshold) do={
        :log info "$upsPrefix UPS input voltage is $voltage"
      }

    :local online ([monitor $item once as-value]->"on-line")
      :if (! $online) do={
        :log info "$upsPrefix UPS is offline"
      }

}

[rextended]

@Josephny

:local iupsname [get $item name]

You can omit that, if is unused

[Josephny]

It’s not used, but I left it in so I would know it (“name”) is an option.

[rextended]

@Josephny

So much to learn!

You can also read my snippets:

Rextended Fragments of Snippets

Over the years I have published dozens of scripts, most need to be updated and are hard to find with standard search. And they also need to be checked if they are still valid for the new stable v7 version. I’m slowly re-reading all of my 10000 posts and I’ll add here all the useful Snippets I found, with a description and the link to the topic where they are present. In the future, when I’m done, I’ll index everything for better search.

[teslasystems]

You’ve made your code overloaded with lots of unneeded stuff.

1. For what reason are you using :foreach cycle, if you have only one UPS on every device?
2. You are (again) reading UPS data twice.
3. If you left ups name (or leaving anything else in the code) just to “know” about it, but it’s not actually used, then comment it, so it won’t be executed.

:local upsPrefix "[UPS]:"

#:local iupsname [get $item name]
:local threshold 115

:local upsData [/system ups monitor ([find]->0) once as-value]; # Read UPS data ONCE
:local rawvolt ($upsData->"line-voltage")
:local voltage ($rawvolt / 100)

:if ($voltage < $threshold) do={
    :log info "$upsPrefix UPS input voltage is $voltage"
}

:local online ($upsData->"on-line")
:if (! $online) do={
    :log info "$upsPrefix UPS is offline"
}

[Amm0]

@rextended

But you wrote two things that shouldn’t be done unnecessarily:
unnecessarily opening { } after a path,
you used ups as the variable name, which is already the path name.

Just for clarity:

1. Only attribute names are problematic, paths/dir are not variables nor passed anywhere.
2. Perhaps not best choice using { } after a path in the example. But it's perfect valid and does create a new scope for :local - something OP asked about.

But the problem here is we go from "I have one UPS" to "I need a :foreach" somehow. There is no need for [find] if it's one static UPS. The .id will always be same and safe.

[rextended]

For
.1 : correct, but better habit to not use at all name already defined…
.2 : correct, but better habit to not create useless nested scopes…

@teslasystems
.1: The foreach | [find]->0 is present because seem that on some device the ups name change, and is for not hardcode name or .id,
AND, if is not present at all one UPS, foreach do not do nothing…
.2: correct…
.3: correct…

[teslasystems]

In case, if there will be no any UPS (for example, some cable issue or whatever), I think it’s better to use :onerror handler, so the script will know about issue and can report about it.
:foreach will behave silently and won’t report about issue.

[rextended]

Untested, i do not have UPS…

[jaclaz]

Only as a side-side note, IMHO “static” variable assignments, (threshold, logprefix) should normally be outside any loop, they are assigned once at script start and not-reassigned at each loop iteration, of course since here there will be only one iteration it doesn’t change anything.

[rextended]

you said it right, but in this case the cycle is an end in itself

[Amm0]

‡ above is for use in /system/script — wrap it { #rextended-code# } if you’re pasting to CLI.

[rextended]

# { ; # on Terminal remove first #
:local logPrefix "[UPS]:"
:local noUPSmsg  "$logPrefix NO ACTIVE UPS FOUND"
:local theshold  115
:local rawread   [:toarray ""]
:local voltage   0
:local isonline  false

/system ups
:local findquery [find where disabled=no]
:if ([:len $findquery] = 0) do={
   :log error $noUPSmsg ; :error $noUPSmsg
}

:foreach item in=$findquery do={
    :set rawread  [monitor $item once as-value]
    :set voltage  (($rawread->"line-voltage") / 100)
    :set isonline ($rawread->"on-line")

    :if ($voltage < $theshold) do={ :log info "$logPrefix input voltage is $voltage" }

    :if ($isonline != true) do={ :log info "$logPrefix UPS is offline" }
}
# } ; # on Terminal remove first #

[teslasystems]

I would make it like this:

:local upsPrefix "[UPS]:"

#:local iupsname [get $item name]
:local threshold 115

:local upsData [/systems ups monitor [find] once as-value]
:local upsCount [:len $upsData]

:if ($upsCount = 1) do={
    :local rawvolt ($upsData->0->"line-voltage")
    :local voltage ($rawvolt / 100)

    :if ($voltage < $threshold) do={
        :log info "$upsPrefix UPS input voltage is $voltage"
    }

    :local online ($upsData->0->"on-line")
    :if (! $online) do={
        :log info "$upsPrefix UPS is offline"
    }
} else={
    :if ($upsCount = 0) do={
        :log info "$upsPrefix UPS not found"
    } else={
        :log info "$upsPrefix Something weird is going on..."
    }
}

UPD: @rextended ohh, you already posted similar idea

[rextended]

I do not do a blind find, disabled UPS do not return line-voltage or if is on-line or not…

[find where disabled=no]

Anyway, now, to put it in Italian, we’re p1ss1ng outside the pot…

If a serious, universal script is to be made, all the errors must also be handled… :lol:

[jaclaz]

@Teslasystem
You are using way too many if/else’s for my personal tastes, of course in this specific case it won’t matter, but in more complex scripts they are likely to bite you back.

@all
What happens if the UPS is NOT found to variable “voltage”?
It is declared as 0 at the beginning of the script, but what value (if any) does it get if NO UPS is connected?
In (say) batch, I would NOT define the variable at all at the beginning, and then use
IF DEFINED
if it is defined, then look at its value, otherwise it means that no UPS was found and there is no real need of checking “on-line” as it clearly it isn’t.
(just thinking aloud)

[teslasystems]

Ahhh, why almost everyone spells my nickname wrong

@jaclaz

You are using way too many if/else’s

May be, but how else to compare UPS count with 3 different values/ranges? We don’t have switch/case, unfortunately.

@jaclaz

What happens if the UPS is NOT found to variable “voltage”?

In my example nothing, the code block with it won’t be executed at all.

In @rextended code it won’t be executed either, because the script will be stopped by :error command.

[rextended]

@jaclaz
Read my previous post...

@rextended

I do not do a blind find, disabled UPS do not return line-voltage or if is on-line or not…
Anyway, now, to put it in Italian, we’re p1ss1ng outside the pot…
If a serious, universal script is to be made, all the errors must also be handled… :lol:

[jaclaz]

@Teslasystems
Sorry I was thinking as you as a single person, not as a plural.

If I get the whole thing right in your code, you are interrogating the array to get the value of $online:

:local online ($upsData->0->"on-line")

but then the check:

:if (! $online) do={
:log info “$upsPrefix UPS is offline”
}
} else={

is for its existance only and you don’t use its value at all, I was saying that you could use the $voltage instead, something like:

:if (! $voltage) do={
:log info “$upsPrefix UPS is offline”
}
} else={

saving one variable.

Of course if I understand what if (! $variable) does correctly.