Skip to content

Merge write branch; add write multiple method.#30

Open
sdghchj wants to merge 40 commits into
alexbeltran:masterfrom
sdghchj:master
Open

Merge write branch; add write multiple method.#30
sdghchj wants to merge 40 commits into
alexbeltran:masterfrom
sdghchj:master

Conversation

@sdghchj

@sdghchj sdghchj commented Jun 22, 2020

Copy link
Copy Markdown

Merge write branch;
Add write multiple method.
Refactor and extract datalink.

TODO: mstp to be implemented.

@alexbeltran

Copy link
Copy Markdown
Owner

Thanks for taking the time adding some of the implementation that I haven't gotten around too. I'll look it over as soon as I can and provide feedback. Are you testing on a bacnet controller?

@alexbeltran alexbeltran self-requested a review July 8, 2020 07:16
@sdghchj

sdghchj commented Jul 8, 2020

Copy link
Copy Markdown
Author

@alexbeltran
I need a golang driver lib of bacnet, but found few available ones on github.
I really appreciate your startup and work of this project, however about 2 years have passed since your last commit. so I have to add some implementation by myself, refer to a C driver .
I have no real testing environment, only tested it with bacsvr, an ip server provided by that C driver.
I know there is a lot of work to do to improve this driver, but really expect its full version.

@alexbeltran alexbeltran left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First pass, there are still 17 files I haven't looked at.

Comment thread encoding/decoder.go
Comment thread encoding/decoder.go
Comment thread encoding/context_tag.go
Comment thread encoding/context_tag.go
import "github.com/alexbeltran/gobacnet/types"

/* returns the fixed tag type for certain context tagged properties */
func tagTypeInContext(property types.PropertyType, tagNumber uint8) uint8 {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. :)

Comment thread datalink/mstp.go
@@ -0,0 +1,95 @@
// +build MSTP

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a WIP. Before removing this tag, I would like to remove the cgo dependency to keep this library pure go. For now this is fine though since I imagine that is a bit of work.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is a WIP that you want useable, could you add this to the readme as well.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by WIP?
MS/TP is another datalink based on serial port 485 and its implemention seems very complex, so I use a C library directly for a temporary solution. To be implemented by pure go.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can ignore this file.
If not set -tags MSTP, this file won't be compiled

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

Successfully merging this pull request may close these issues.

2 participants