Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Steps toward support of protocol revision 22 #485

Closed
calligp opened this issue May 3, 2023 · 2 comments
Closed

Steps toward support of protocol revision 22 #485

calligp opened this issue May 3, 2023 · 2 comments

Comments

@calligp
Copy link

calligp commented May 3, 2023

We are trying to certify a device embedding the Bacpypes library version 0.18.6. We would like to claim protocol revision 22.
Do you have any restriction in mind about this, regarding the state of the library ?

During the investigation, we have found some gaps between the current library implementation and the 2020 standard.

First, do you still maintain the implementation in py25 directory.
Indeed, I have found discrepancies between enumerations and other type declarations in this directory and in the other ones.
Just in case, I have created the following patches for all three implementations.

If pull requests would be more convenient for you, I can also use that method to provide you with my suggested changes.

  1. An error code is missing in the list of defined errors: "duplicateEntry (137)".
    This error has been introduced in the 2016 standard to prevent writing the same time more than once in a daily schedule (in a weekly or exception schedule).
    I have also checked the complete list of error codes managed by the library, and a lot of new ones added to the standard for BACnet/SC were not present yet.
    You will find the corresponding update in the following patch:

0006-Add-new-error-code.txt

  1. Some properties used for the Network Port object do not have the identifier declared for them in the standard.
    It is probably only a typing error. Here are the patches I am suggesting:

0004-Fix-id-error-in-property-identifier-list.txt

  1. Additionally, I have seen that new objects added with protocol revision 22 have already been implemented,
    but the corresponding enumeration (ObjectTypesSupported) is not complete.

0001-Add-new-object-type-to-support-revision-22.txt

  1. I have also seen that the list of supported services is not complete either.

0002-Add-new-service-to-support-revision-22.txt

  1. As you probably know, the Network Port object is mandatory for any device using BACnet/IP and claiming a protocol revision superior to 19.
    We have discovered that some of its property types are not encoded as described in the standard, because Wireshark was unable to decode them.
    For example, we have tried to read and write the HostNPort structure (property fdBBMDAddress of the Network Port object), and Wireshark interpreted it as a proprietary property.
    image

The following patch makes it so the structure can be interpreted by Wireshark:

0003-Fix-encoding-of-some-network-structure.txt

  1. To complete support of the Network Port object, a new state has been added for ReinitializeDevice request to activate pending changes.
    This state is also a restart reason that must be notified. For this purpose, the reason must be added in the RestartReason enumeration.
    I have also added a new Reliability value (multiStateOutOfRange (25)).
    The following patch should take care of this:

0005-Add-missing-value-in-enumeration.txt

@JoelBender
Copy link
Owner

Thank you for your interest in BACpypes, and thank you for the patches.

We would like to claim protocol revision 22. Do you have any restriction in mind about this, regarding the state of the library?

Apart from encoding and decoding PDUs, I have not run a particular client or server application against any BIBBs to confirm that all of the pieces are implemented correctly. The easy ones (like the old Conformance Classes) have been used by lots of people, but I have reservations about the more advanced requirements. Client applications (A-side) are much easier to build and test than servers (B-side) which usually have lots of additional code for the real-world application.

As you probably know, the Network Port object is mandatory for any device using BACnet/IP and claiming a protocol revision superior to 19.

When the async/await features came to Python I started a re-write of the library BACpypes3 and that has an IPv4 implementation of a BACnet Application NPO. The client API is much easier to use, like await app.read_property(...), and entire chunks of code like the task manager and IO control blocks are gone. The IPv4 stack layer is stable, the IPv6 stack pieces should also be stable (slightly less confident), and the beginnings of SC (encoder/decoder layered on websockets, but not the SC-hub or SC-switch).

So I appreciate the patches and I will apply them, but the vast majority of the development will be on BACpypes3. If we can get it up-to-date with the 2020 publication that would be a great victory!

We have discovered that some of its property types are not encoded as described in the standard, because Wireshark was unable to decode them.

I consider that a pretty bad problem, thank you for the patch, and I'll port the fix into BACpypes3 if its needed.

First, do you still maintain the implementation in py25 directory.

No, I stopped updating it when it was no longer easily available. With the docker world, maybe images like the ones here can be used and could come back to life...interesting thought.

I have found discrepancies between enumerations and other type declarations in this directory and in the other ones.

Yep, I'm not surprised, and there might be substantial differences in the supported services as well.

I have created the following patches for all three implementations.

Thank you!

JoelBender added a commit that referenced this issue May 10, 2023
JoelBender added a commit to JoelBender/BACpypes3 that referenced this issue May 10, 2023
@JoelBender
Copy link
Owner

These patches have been applied and are released in 0.18.7 and in BACpypes3 0.0.73. Thank you again!

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants