I'm not going to paste the code in the forum, because you really need the bacnet4j source so that you can test it out, download the source from http://sourceforge.net/projects/bacnet4j/files/bacnet4j/1.3/
and make sure you don't have any problems with running all the tests.
And as I said before, it's been over a year since I touched bacnet4j, so good luck, I'm sure once you have the source, you'll figure out what is going wrong.
I haven't used BACNet4j for over a year, but from the code you provided, it doesn't look like you have another bacnet device that would be responding to the requests. You'd need to have a BACnet "server" or slave device actually sending the values.
For example, to get this test to work:
You need to have this test already running in the background:
I'd recommend that you start by getting the samples working, before trying other things.
Hi Taresh,
If you're talking about the analog COV subscription changes, that was merged in Nov 2012, see: http://forum.infiniteautomation.com/forum/posts/list/1207.page
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
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.
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.
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
As far as I can tell it is the BACnet Object Identifier that must be unique. Although I have noticed that some third party BACnet devices get a bit confused when they see objects that have the same name. Although bacnet4j only cares about the object identifier being unique.
I hope that helps
Hi Gatos,
We're still in discussions with our client if they want us to implement COV subscriptions for analog values.
A simple workaround or "dirty hack" is simply to add
supportedObjectTypes.add(ObjectType.analogValue);
supportedObjectTypes.add(ObjectType.analogInput);
supportedObjectTypes.add(ObjectType.analogOutput);
to ObjectCovSubscription
That will make it accept the COV subscriptions, and it will send COV notifications, however, it will send a COV notification for any change of the analog value. Which for analog values is probably a bad thing, because they could be changing all the time. The real solution needs to implement some sort of threshold, so that COV notifications are only sent once the value changes enough.
Hi kishorev,
You seem to be using the bacnet4j api in a bit of a strange way. But I can see a few things that might help. First of all before you ask for the object list, you probably need to do a broadcast, simply so that the responding bacnet device knows you exist, otherwise I'm pretty sure it can't respond to you.
You can do it with a broadcast:
localDevice.sendBroadcast(47808, localDevice.getIAm());
Or you can send it a direct message, since you know who you are talking to:
localDevice.sendUnconfirmed(remoteDevice.getAddress(), null, localDevice.getIAm());
Then then other problem is the way you are getting the object list, you are really using internal API methods, you are much better off using:
List<ObjectIdentifier> oids = ((SequenceOf<ObjectIdentifier>) localDevice.sendReadPropertyAllowNull(remoteDevice, remoteDevice
.getObjectIdentifier(), PropertyIdentifier.objectList)).getValues();
Where remoteDevice is an instance of a RemoteDevice, in your case:
Address address = new Address(null,BACnetUtils.dottedStringToBytes("192.168.1.3"),47808);
RemoteDevice remoteDevice = localDevice.findRemoteDevice(address, null, deviceId);
Hi Gatos,
I think the SlaveDeviceTest needs to know about the SimpleSubscriptionClient before it will accept COV requests from it. I found that if you start the SlaveDeviceTest, then the SimpleSubscriptionClient, stop the SimpleSubscriptionClient, comment out the sendBroadcast and then start it again. COV requests still work, that is because the slave device already knows about the SimpleSubscriptionClient.
An alternative to a broadcast would be to send a direct iAm to the slave device by adding this after line 25:
localDevice.sendUnconfirmed(d.getAddress(), null, localDevice.getIAm());
I'm still pretty new to BACnet myself, so I'm not sure the technical reason why simply attempting to subscribe by COV, doesn't implicitly add the device to the SlaveDeviceTest.
Hi Gatos,
Download the source code again, the source was broken for few days when Matthew fixed the [url=http://forum.infiniteautomation.com/forum/posts/list/1242.page]UDP 7 segments bug
Hi Gatos,
The SimpleSubscriptionClient is a test class, you need go and download the latest source to get at it.
http://bacnet4j.cvs.sourceforge.net/bacnet4j/ then click Download GNU tarball
Or download from CVS using the instructions here: http://sourceforge.net/scm/?type=cvs&group_id=224576
To get the test working, simply start the SlaveDeviceTest, and then run the SimpleSubscriptionClient in another window and you should see COV's coming through. I've run those classes in my Eclipse and it worked fine for me.
Cheers,
-Joel
Hi Matthew,
You missed the MORE_THAN_64 segments in the valueOf function, which makes bacnet4j pretty unusable
This is the diff of what's missing:
diff --git a/src/com/serotonin/bacnet4j/enums/MaxSegments.java b/src/com/serotonin/bacnet4j/enums/MaxSegments.java
index 0845c74..362e210 100644
--- a/src/com/serotonin/bacnet4j/enums/MaxSegments.java
+++ b/src/com/serotonin/bacnet4j/enums/MaxSegments.java
@@ -67,6 +67,8 @@ public enum MaxSegments {
return UP_TO_32;
if (id == UP_TO_64.id)
return UP_TO_64;
+ if (id == MORE_THAN_64.id)
+ return MORE_THAN_64;
throw new IllegalArgumentException("Unknown id: " + id);
}
Cheers,
-Joel
Thanks that now breaks the AnnexFEncodingTest though, I also wrote a test that tries to use 4000 objects as well, should I submit that as a patch for you?
@mlohbihler said:
Not sure what you mean. This is the code that i have (lines 274 and 275):
if (segmentsRequired > request.getMaxSegmentsAccepted() || segmentsRequired > 128) throw new BACnetException("Response too big to send to device; too many segments required");BACnet4J will respond with up to 128 segments unless limited further by the max accepted by the client.
That is true, however request.getMaxSegmentsAccepted() is a maximum of number 7 and not 128.
Look at lines 240 - 258 of ConfirmedRequest, maxSegmentsAccepted is set directly from the byte queue, which works for everything except the one use of getMaxSegmentsAccepted. It needs to work exactly like maxApduLengthAccepted, which is the solution I came up with. I created an enum that maps between the integer/byte value and the actual meaning of that value.
ConfirmedRequest(ServicesSupported servicesSupported, ByteQueue queue) throws BACnetException {
byte b = queue.pop();
segmentedMessage = (b & 8) != 0;
moreFollows = (b & 4) != 0;
segmentedResponseAccepted = (b & 2) != 0;
b = queue.pop();
maxSegmentsAccepted = (b & 0x70) >> 4;
maxApduLengthAccepted = MaxApduLength.valueOf((byte) (b & 0xf));
invokeId = queue.pop();
if (segmentedMessage) {
sequenceNumber = queue.popU1B();
proposedWindowSize = queue.popU1B();
}
serviceChoice = queue.pop();
serviceData = new ByteQueue(queue.popAll());
ConfirmedRequestService.checkConfirmedRequestService(servicesSupported, serviceChoice);
}
Lines 191-193 show that no conversion is taking place, it's still the original byte/integer value.
public int getMaxSegmentsAccepted() {
return maxSegmentsAccepted;
}
Hi Matthew,
We found a bug where com.serotonin.bacnet4j.npdu.ip.IpMessageControl.sendResponse(InetSocketAddress, Network, ConfirmedRequest, AcknowledgementService) was limiting the number of UDP packets to 7 segments instead of more than 64. We need to have at least 2000 BACnet Objects on a local device and it was complaining about too many segments.
I have created a patch for this, how should I give this to you?
For my own convenience I have converted your CVS repository to GIT, so if you like I could put the repository up on Github, then I could fork your repository and create pull request. Otherwise I can just upload a patch in the [url=http://sourceforge.net/tracker/?group_id=224576&atid=1062318]Patches section of sourceforge
Let me know what you prefer.
Cheers,
-Joel
Hi Matthew,
We found a bug where com.serotonin.bacnet4j.npdu.ip.IpMessageControl.sendResponse(InetSocketAddress, Network, ConfirmedRequest, AcknowledgementService) was limiting the number of UDP packets to 7 segments instead of more than 64. We need to have at least 2000 BACnet Objects on a local device and it was complaining about too many segments.
I have created a patch for this, how should I give this to you?
For my own convenience I have converted your CVS repository to GIT, so if you like I could put the repository up on Github, then I could fork your repository and create pull request. Otherwise I can just upload a patch in the [url=http://sourceforge.net/tracker/?group_id=224576&atid=1062318]Patches section of sourceforge
Let me know what you prefer.
Cheers,
-Joel
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.