Add alarm support

[tomrosenback]

Adds alarm support for both MQTT and HTTP API.

Still work in progress, but please have a look and let me know if you want anything specific changed. Worth to note, first in this branch is untested.

Fixes issue #31 when merged

[Jalle19]

@tomrosenback I think we should treat the list of alarms in two different ways:

1. The HTTP API would return a list of triggered alarms, along with the date/time they were triggered and whether they're dismissed or not.
2. The MQTT binary sensors would be one per alarm type, and be on when the alarm has occured and is not dismissed, otherwise off.

The problem with the current (albeit draft) implementation is that it doesn't allow for the same alarm to happen twice. IMO that's the normal case, e.g. you end up with the same "tuloilma kylmä" for example couple of times per winter when your LTO roller freezes shut.

What do you think?

[tomrosenback]

For HTTP API, sure, I can agree with that, but isn´t the most recent alarm of any type the most interesting? What I mean is if say "tuloilma kylmä" has an active state, we are only interested in knowing that? I at least go and reset any errors after issue is fixed so the list of errors is mostly empty.

Also in regards of dismissed or not, isn´t that basically the same thing, it takes a short while if the problem does not persist and the error goes away, so with that said as on MQTT side only interesting statuses are ON, and OFF (dismissed goes under this also).

To conclude

1. HTTP API; You mean would want all alarms here that are active (only ON), repeated alarms aswell? This could of course be implemented as an array of dates, to make it not differ too much from how alarms are handled on MQTT.
2. This is the way it is implemented already, or did I miss something? Date when last active could be push as an attribute aswell here.

@Jalle19 What do you think? Am I totally off?

[Jalle19]

For the HTTP API I'm thinking something like this:

"alarms": [
  {
    "code": "TE10",
    "name": "TE10InletAfterHeaterCold",
    "description": "Tuloilma LTO:n jälkeen kylmä",
    "timestamp": "2022-01-18T12:58:05Z+0200"
  },
  { ... },
]

I.e. if there are no alarms then the list is empty.

2. This is the way it is implemented already, or did I miss something? Date when last active could be push as an attribute aswell here.

Yes, I guess that's true.

In this case I think it would make sense to use different approaches for the HTTP and MQTT APIs. MQTT is more limited in being simply "on" or "off", while HTTP can expose the alarm history as well (which is basically what you get on the control panel as well).

[tomrosenback]

HTTP API, ok with me.

MQTT, ok.

Will make adjustments accordingly,

[Jalle19]

I'm not entirely sure what a Pandion machine looks like, but on my Pingvin it's easy to trigger an alarm by removing the LTO drum temporarily.

[tomrosenback]

Biggest difference I guess is size.

[tomrosenback]

Please test this out, have tested both HTTP and MQTT API and to me it looks ok now.

[tomrosenback]

Seems to throw unhandled promise rejections now, any idea?

[Jalle19]

Seems to throw unhandled promise rejections now, any idea?

Can you show me the logs from when it happens?

[tomrosenback]

Seems to throw unhandled promise rejections now, any idea?

Can you show me the logs from when it happens?

Doesn´t seem to happen anymore, no changes to code...

[tomrosenback]

Seems to throw unhandled promise rejections now, any idea?

Can you show me the logs from when it happens?

Doesn´t seem to happen anymore, no changes to code...

`node eda-modbus-bridge.mjs --unhandled-rejections=warn -d /dev/ttyUSB0 --http false -m mqtt://localhost:1883

... discovery messages ... ... callback messages ... ... random idle duration ...

node:internal/process/promises:265 triggerUncaughtException(err, true / fromPromise /); ^

[UnhandledPromiseRejection: This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason "#".] { code: 'ERR_UNHANDLED_REJECTION' }

Node.js v17.4.0 `

[Jalle19]

I think the rejection warnings is a bug in modbus-serial, but I haven't been able to track it down 🤷

[tomrosenback]

Looks good now, thanks a lot! I'll test this locally ASAP and merge if it works.

You are welcome! I have mock tested both MQTT and HTTP locally, and it works, currently no alarms in my system so can´t tell if it really works, but if you have an easy way to test, please do to verify it with a "real" alarm.

[Jalle19]

Sorry, I accidentally pushed directly to your branch when I thought I was pushing to my remote. Didn't know I had write access :D

[Jalle19]

Probably best if you force push your changes again

[tomrosenback]

Sorry, I accidentally pushed directly to your branch when I thought I was pushing to my remote. Didn't know I had write access :D

No worries.

[tomrosenback]

Probably best if you force push your changes again

What I can see your changes are not here anymore.

[tomrosenback]

Probably best if you force push your changes again

What I can see your changes are not here anymore.

You have started from my add-alarm-support branch, which made it show up here.

[Jalle19]

@tomrosenback I took your changes for a spin. Turns out my alarm history was full (all 20 slots populated). There were some weirdness with null values in the array, and I figured it would make more sense to expose an "alarm history" through HTTP and "alarm states/statuses" over MQTT. I made some changes so that the MQTT part (alarm statuses) are determined based on the alarm history (no historic active alarm == no current active alarm, since it's all in the history), can you try this branch: https://github.com/Jalle19/eda-modbus-bridge/tree/alarm-test ?

This has the added benefit of being (IMO) simpler since there's no onlyActive and distinct booleans to keep track of.

[tomrosenback]

Works, and looks good to me. My history is also full so can´t say what an empty slot would look like.

Less is more, removing onlyActive and distinct with added benefits sounds very good.

[tomrosenback]

Easiest for now would be to merge this PR, and then add a new PR for your changes? Or then ditch this PR all together and just use your branch? Which ever is fine.

[Jalle19]

Let's merge this so the author information is not lost, I'll then PR my changes on top of this.