Guidance for implementing COV for Analogs

[mlohbihler]

I haven't been in that code for a while now, but it sounds like that approach should work.

[japearson]

Hi Matthew,

Thanks for the quick response, we'll head down that path and submit a patch once we have it working.

[japearson]

For what it's worth it looks like at this stage our client doesn't want COV for the moment, so we may not end up submitting a patch after-all. However they may change their mind, but at least the research is up there for anyone that really wants to implement COV for Analogs but isn't sure what's involved.

[japearson]

Hi Matthew,

Our client did need COV for analogs in the end. I have just implemented it and submitted a patch on sourceforge: https://sourceforge.net/tracker/?func=detail&aid=3581790&group_id=224576&atid=1062318

Can you take a look at it and let me know if you want me to change anything?

Thanks,

-Joel

[mlohbihler]

Hi Joel,

The patch looks ok. I don't have any equipment upon which to test it at the moment though. One thing: why create a static class (ThresholdCalculator) with only static methods? Logical encapsulation?

[japearson]

Hi Matthew,

Yes it was just for logical encapsulation and it made it easier to test. However if you would like me to move ThresholdCalculator to be a normal class, or just move its methods into ObjectCovSubscription. If I move the methods into ObjectCovSubscription, I should probably make isValueOutsideOfThreshold private, so the testing will be a little more fiddly, but still relatively straight forward.

Or if there is some other style you prefer, just let me know, I'm happy to change it to suit whatever coding style you prefer.

In terms of testing on real equipment, next week we'll be testing it with some Siemens hardware, so I can tell you the result of it. But then obviously you'd just be taking my word for it.

[mlohbihler]

No problem, Joel. It's fine the way it is. Let me know how the testing goes and i'll check the changes into CVS.

[japearson]

Hi Matthew,

Our testing with the Siemens BMS went off mostly without a hitch, so I'd say the COV code is good to merge.

The only minor issue we came up against, was when we restarted our bacnet4j connector, we couldn't accept new COV subscriptions, because we didn't know about the remote device.

So the workaround we came up with was sending out a "WhoIs" request before failing the COV subscription. There was probably a better way but this is what we did:

The original code from com.serotonin.bacnet4j.obj.BACnetObject.addCovSubscription(Address, Network, UnsignedInteger, Boolean, UnsignedInteger)

                if (confirmed) {
                    // If the peer wants confirmed notifications, it must be in the remote device list.
                    RemoteDevice d = localDevice.getRemoteDevice(from);
                    if (d == null)
                        throw new BACnetServiceException(ErrorClass.services, ErrorCode.covSubscriptionFailed,
                                "From address not found in remote device list. Cannot send confirmed notifications");
                }

We changed to:

                if (confirmed) {
                    // If the peer wants confirmed notifications, it must be in the remote device list.
                    RemoteDevice d = localDevice.getRemoteDevice(from);
                    if (d == null) {
                        // Send a WhoIs before sending a failure message, so that subsequent subscriptions will hopefully work
                        try {
                           this.localDevice.sendUnconfirmed(from, network, new WhoIsRequest());
                        }
                        catch (BACnetException e) {
                           // If the WhoIs request fails we just ignore it.
                        }
                        throw new BACnetServiceException(ErrorClass.services, ErrorCode.covSubscriptionFailed,
                                "From address not found in remote device list. Cannot send confirmed notifications");
                    }
                }

Although I don't know if you'd want to merge something like that change, as it is more of a workaround and a bit specific to how our environment is setup.

Other than that we verified that COV subscriptions for Analog devices worked flawlessly taking into account their threshold.

[mlohbihler]

I agree that the code appears more of a workaround than standard practice. Also, will not the BacnetServiceException be thrown no matter what? Shouldn't there be a subsequent check (after some delay) to see if the remote device is now known?

Thanks for the update in any case. I will check in the code as it is.

[japearson]

Hi Matthew,

Yes the BacnetServiceException will still be thrown, but the Siemens BMS software, goes into some aggressive polling mode when it receives a COV subscription failure. Every 5 seconds it continually tries to resubscribe, so even though the exception still bleeds through and sends a BACnet COV failure message. 5 seconds later the next COV subscription request succeeds.

Although later I was thinking a possibly better way to do it would be to accept the subscription anyway, but when sending a notification, do the check if we have the remote device yet, and simply not send a notification if we can't look up the remote device.

However we had already tested the workaround against the Siemens hardware so we decided to leave it the way it was.

Cheers,

-Joel

[mlohbihler]

If the remote device you want isn't found from a WhoIs, but you know it's there, you can always discover it manually using LocalDevice.findRemoteDevice.