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

Introduce runtime API for navigating around Sunspec device models #5

Merged
merged 7 commits into from
Dec 2, 2016

Conversation

jonseymour
Copy link
Collaborator

Hello @crabmusket, this pull request is a work in progress - not to be merged at this stage.

The intent is to provided you with visibility about where I am headed and give you an opportunity to provide feedback about that direction.

My intent is to provide an abstract API for navigating a Sunspec device (or, indeed, array of sunspec devices) irrespective of the physical medium in which the device is implemented. So the same application code should work whether the physical medium is a Modbus RTU (or TCP) connection or a Sunspec XML document or an in-memory stub used for unit testing purposes.

Blocks() int // Answer the number of blocks

Block(i int) Block // Answer the nth Block.
Do(func(b Block)) // Iterate across all the blocks, in order.
Copy link
Owner

Choose a reason for hiding this comment

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

How I long for a proper type system (which would allow collecting results) :p

type Array interface {
Device(id string) Device // Answer the specified device instance or nil
Do(func(d Device)) // Iterate over all the devices.
}
Copy link
Owner

Choose a reason for hiding this comment

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

Is there a more descriptive name than Array? Or is this directly from the spec?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The purpose of this type is to model the collection of devices that can appear in an XML document.

I chose the name "Array" based more on usage in "Allen Telescope Array" rather than the traditional computer science meaning of array although, of course, both are related uses.

Copy link
Owner

Choose a reason for hiding this comment

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

Yup. Since Go doesn't have a type called Array we should be fine. Especially as imports will often be qualified.

// An array is a collection of devices.
//
// This type is used when the underlying representation
// is an XML document.
Copy link
Owner

Choose a reason for hiding this comment

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

Isn't this the generic interface?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is - the motivation for the Array type is the fact that the XML format allows multiple devices to be described by one document.

// register for which a scaling factor is defined, then the register will be
// multiplied by the scaling factor to produce the expected 64 bit float result.
// Note that both the point and the related scaling factor point (if any) must
// be error free at the point the scaling factor is applied or a panic will result.
Copy link
Owner

Choose a reason for hiding this comment

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

What does error-free mean in this context?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The Point type is designed to be type safe and not fiddly to use (e.g. no error checking on each any every access to a point). The way that it is achieves this is to keep track of both a value and an error. If the error is not nil, then the point is in an error state and any attempt to use its value will result in a panic. A point starts out in an uninitialised error state and moves to an error free state when a value is associated with the point, usually after a Read.

Provided you have done enough to ensure that the point is present (by issuing a successful Read call) and you choose the correctly typed accessor method, it will not panic. This is the "error free" case.

However, if you fail to do these things then it will panic. In particular if you try to get the scaled float64 value of a point that has a scale factor, then it will panic if either the point is in an error state or it's related scale factor point is in an error state, perhaps because it hasn't been read yet.

Copy link
Owner

Choose a reason for hiding this comment

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

Ok, sounds good. Sounds like the most common error would be use-before-read which can be clearly explained in docs.

@crabmusket
Copy link
Owner

I wholeheartedly support this goal 👍

Since you're doing all the work, how about I just add you as a collaborator to this repo? We could still work with a branch-and-merge model if you want to run everything by me, but I sense my input will be limited.

That or we make your fork the 'blessed' repo.

@jonseymour
Copy link
Collaborator Author

I am happy to continue with branch and merge or for me to become a collaborator on your repo or a combination of both - whatever is easiest for you

@crabmusket
Copy link
Owner

crabmusket commented Nov 28, 2016

I've added you as a collaborator. Whether you want to move development to your own repo is at your discretion, of course :).

@jonseymour
Copy link
Collaborator Author

@crabmusket thank you - I'll push something over the next day or two. The next increment is a fairly large one and I want to get it largely right before I push it.

@jonseymour
Copy link
Collaborator Author

jonseymour commented Nov 29, 2016

This is still a work in progress. I almost have a working memory-backed implementation of the API - I just need to implement the Read and Write methods of the memory/device type and then write some tests to check that everything is sane.

@jonseymour
Copy link
Collaborator Author

jonseymour commented Nov 29, 2016

This update has a (at least partially) working memory backed implementation with a single test case.

The memory backed implementation lays out a slab of memory (actually a byte slice) in the same way as it would be in a Modbus implementation. Once this is done the device can be "opened" against that slice using memory.Open([]byte)

A list of things I am doing to work on in the near future:

  • make Block.Point() return an "error point" to improve ease of use
  • force Block.Read() to also read related scale factors even if not specified
  • force Block.Read() to invalidate Points not read if the related scale factor is read
  • force invalidation of related points when scale factor is set
  • handle zero-terminated sunspec strings correctly
  • passthrough the Pad type
  • put bounds checking on point access beyond block length (e.g. model 1)
  • undo error point changes, introduce MustPoint() and MustBlock() methods as well to be more inline with go idioms
  • simplify details of PointImpl
  • create a rich test model with repeats and all field types
  • many more tests
  • documentation
  • support for a Modbus device
  • support for an XML device
  • squash into a smaller number of commits, prior to merge
  • fixup inconsistencies in some type labels

@jonseymour
Copy link
Collaborator Author

jonseymour commented Nov 30, 2016

@crabmusket - the series - 75cbb0c through 16d08e7 (which is tree same to f86e08c) is a rebasing of all the previous work with some squashes. It is what I intend to merge into master once you have had a chance to do a review.

In parallel I intend to flesh out some more unit tests for the work done so far.

@jonseymour jonseymour changed the title WIP - do not merge Introduce runtime API for navigating around Sunspec device models Dec 1, 2016
@jonseymour
Copy link
Collaborator Author

@crabmusket I have eliminated the development branch. This branch is ready to be merged to master in its current form. If you would like a chance to review it, let me know, otherwise I will do the merge towards the end of today (Friday).

@@ -24,7 +24,7 @@ var types = []Type{
Type{typelabel.Count, typelen.Count},
Type{typelabel.Enum16, typelen.Enum16},
Type{typelabel.Enum32, typelen.Enum32},
Type{typelabel.Eui48, typelen.Eui48},
Type{typelabel.EUI48, typelen.EUI48},
Copy link
Owner

Choose a reason for hiding this comment

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

Stylistic complaint, I pref acronyms to maintain camel case (e.g. Ipv6 and Uint below)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, I reverted the Eui48 changes.

@crabmusket
Copy link
Owner

crabmusket commented Dec 2, 2016

This is fantastic work. Love the addition of tests. I don't have the bandwidth right now to actually review the code, but I'm happy for you to merge when you're ready :).

Signed-off-by: Jon Seymour <jon@wildducktheories.com>
The API is configured by the SMDX models and provides a uniform
mechanism for navigating around a strongly typed representation
of a Sunspec device.

It is intended that different that same API can be used irrespective
of whether the actual device is a simulated (in-memory) device, a Modbus
connected device or an XML document containing a copy of device data.

The API knows about scaling factors and helps ensures that scaling factors
and values are read from the underlying device in the same call, relieving
the programmer of some of this complexity.

Signed-off-by: Jon Seymour <jon@wildducktheories.com>
The SPI interfaces are used by device implementations to acquire
additional access to the underlying API implementation without
exposing these details to consumers of the API itself.
The implementation supports the "sunspec" types and also the "spi" types.

Signed-off-by: Jon Seymour <jon@wildducktheories.com>
This package maps a Sunspec device over a byte slice.

The byte slice is laid out in a similar fashion to the
Sunspect Information Model will all multiword values
being laid out in big endian order.

Signed-off-by: Jon Seymour <jon@wildducktheories.com>
@jonseymour jonseymour merged commit 8811540 into crabmusket:master Dec 2, 2016
@jonseymour jonseymour deleted the working branch December 2, 2016 05:21
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