Guidance for implementing COV for Analogs

[japearson]

Hi Matthew,

We're using BACnet4J on a project for work, and as a part of communicating with some 3rd party Siemens software it's come up that we need to support COV for Analogs. We're going to need this is in the next week or so, so we've decided that we're going to implement COV for Analogs ourselves.

I've had a read for the [url=http://mango.serotoninsoftware.com/forum/posts/list/658.page]COV for Analog Input forum post and from what I can tell the main thing that is missing with COV for Analogs is using the covIncrement value to track when something has changed.

So this is how I was thinking of implementing it and let me know if you think there a better way:

Other than adding analogValues as supported object types:

supportedObjectTypes.add(ObjectType.analogValue);
...

I think we would need to add a hashmap to com.serotonin.bacnet4j.obj.ObjectCovSubscription that would keep track of what the last sent value of the supportedPropertyIdentifiers was.

Then I think we'd add a method called isNotificationRequired, that took the newValue, OldValue and the PropertyIdentifier being updated.
If the PropertyIdentifier was presentValue and presentValue was a Real it would then look up the covIncrement value and take into account the value it last sent, the new value and the oldvalue. It would then return a boolean value as to whether a cov change should be sent or not.

Then we'd just hook in the new isNotificationRequired function into either com.serotonin.bacnet4j.obj.BACnetObject.setPropertyImpl(PropertyIdentifier, Encodable) just before com.serotonin.bacnet4j.obj.BACnetObject.sendCovNotification(ObjectCovSubscription, long) or actually inside sendCovNotification itself.

Does this sound reasonable to you?

Do you have any preference as the way to implement it as suggested above?

Thanks,

-Joel

[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.