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

Update BACnetClient.cs #15

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

BennoMeijer
Copy link

Add a overload for the ReadRangeResponse so a bacnetBitString can be used and status flags set to false will be visable

Add a overload for the ReadRangeResponse so a bacnetBitString can be used and status flags set to false will be visable
@gralin
Copy link
Member

gralin commented Dec 19, 2017

@BennoMeijer I remember the discussion we had about the trend log and bit string but is this the right way to go? First of all I would like to avoid putting class like BacnetBitString in the main API of BACnetClient. In version 2 I will simplify it and avoid using classes like this in the API as I consider them more advanced or internal components. How about we use the change from PR #11 and in ReadRangeResponse we declare how many bits are to be used, instead on using the enum value which can suggest we are using only 1 therefor ignoring the remaining false ones.

@BennoMeijer
Copy link
Author

Hello Gralin.
I did not want to change too much, but for the status flags FirstItem, LastItem and MoreItems I also need the flags if they are false. I asked the support center for the scada software and told me that it was mandatory to have these flags. If the ReadRangeResponse is only used for the trendlog, we also could set the bits there and use a uint for the status, but i didn't want to break someone else his code

@gralin
Copy link
Member

gralin commented Dec 29, 2017

@BennoMeijer I get your point and sorry for the delay, I was swamped at work. I will either merge this or suggest a change withing a week. Happy New Year by the way 😄

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

Successfully merging this pull request may close these issues.

2 participants