Original issue edgexfoundry/edgex-go#594

Like it says in the title -- README!!!!

The current Validate() function for the Interval model (found here) validates the Start/End timestamps by attempting to parse them as integers and returning any errors that result. This doesn't work because the expected timestamps contain literal "T" characters.

This code needs to be updated to use the time.Parse function to properly handle these timestamps.

This is the root cause for edgex-go issue #1308: edgexfoundry/edgex-go#1308

I recently reported an issue against device-sdk-go that concerned dumping entire objects in non-debug log messages.

The offending log message was actually including the entire state of a Device object, which is enormous! This got me to thinking whether it's actually a good idea for us to be embedding an entire DeviceProfile in every device as opposed to DeviceProfile.Name? The device SDKs have a cache of device profiles, and to me, it would be much more efficient to always retrieve them from the cache instead of embedding them in every single device.

It also appears that the device-sdk-go callback handler doesn't properly update every device in its device cache when a Profile is updated. It's also not clear who's responsible to update value descriptors if/when a DeviceProfile change occurs. It doesn't look like device-sdk-go currently handles this case.

Note - this is related to issue #40 which proposes removing DeviceService from Device.

There is currently no validation of the Type field of a PropertyValue.
We should add validation logic which enforces that type can only be one of the following values:

• bool
• string
• int8|int16|int32|int64
• uint8|uint16|uint32|uint64
• float32|float64
• binary

Very clearly there is a cut-and-paste error here. This was introduced as part of the model validation PR #66.

You can see the Id property on the Reading is erroneously being assigned to Value as opposed to the Value property during unmarshaling.

Currently when marshaling a DeviceProfile to json or yaml, there are lots of empty fields, which should be great if they are removed.

the following field value is lost when return one device's command metadata
"url": "http://localhost:48082/api/v1/device/fcbacb4f-bc82-4211-8b58-ef2f29f3c0ed/command/b88fb9a9-0126-47cc-bed9-a248fc449f07"

The reason is that the range statement just copys a duplicate value of original object, so it will not change the original object.

I have fixed it, Please see my PR-#92

Service clients wrap the raw HTTP error and associated text response in an ErrServiceClient type in order to preserve and return the status and description of failures when calling dependencies. In the case of 404s, for some reason, this is not being done. This leads to dependent services returning 500 codes when they should return the original failure code, or otherwise make a relevant decision conforming to their RAML definition.

This issue is a small step in support of the issue #27

Issue #27 is too large to do all at once so it will be broken into smaller chunks that are easily described and testable. Also, the workstream cannot impact the larger efforts scheduled for Edinburgh.

In the interest of eventually removing the DeviceProfile from the Device struct, the CommandResponseFromDevice needs to be able to obtain its Commands from a different location than the supplied Device parameter. Currently the property is set like this:

To facilitate backward compatible refactoring, add a parameter to the CommandResponseFromDevice function of type []Command. This new parameter will be used to set the property at the link above.

Per the Core WG call on 14-Mar-19 we need to start adding support for binary readings as part of the CBOR initiative for Edinburgh release. To that end, the following changes need to be made in the core contracts module.

• Add to ValueDescriptor model
  • "MimeType" property (string)
  • Modify unit tests accordingly
• Add to Reading model
  • "Blob" property ([]byte)
  • Modify unit tests accordingly
• Clients/coredata/event.go
  • The "Add" method should be refactored to look at the incoming context for a Content-type header. If found, then send the value through as part of the request. Otherwise default to JSON, which is the current behavior.

Please define a new constant for the new header value application/cbor. This will be usable by any application utilizing the contracts and will avoid naming mismatch issues.

(as discussed dev svcs wg 4-8-19).
Remove models/Units.

1.) Add new method to EventClient interface
EncodeEvent(e models.Event) ([]byte, error)
2.) Modify EventClient.Add([]byte, Context) (string, error)

Based on discussion around this flow edgexfoundry/device-sdk-go#233

The Filter property on the Registration model is being marshaled with the name filter.go. Please fix both the marshaling and unmarshaling.

To make the data type of Attributes of DeviceResource simple, replace map[string]interface{} with map[string]string. interface{} was my suggestion to make the Device Service more complex. In addition, map[string]string meets the C SDK design.

In reviewing my changes for a downstream repo that refactored from edgexfoundry/edgex-go to edgexfoundry/go-mod-core-contracts I noticed that to keep the Attribution.txt consistent we would want a LICENSE file in this repo.

According to edgexfoundry/device-sdk-go#107, we need additional field floatEncoding in PropertyValue and ValueDescriptor

That said, we re-hashed this once again during today's Core WG meeting. The decision was that we'll add a new attribute (strawman name "floatEncoding" to the ValueProperty & ValueDescriptor, which if specified as "printfFloat" (other suggestions welcome, but this is the most accurate I could come up with), then floats will be encoded using the C printf style encoding (e.g. "3.14159265e+00"). If this new attribute isn't specified, then the default will be base64 as currently implemented.

Provision Watcher client is not implemented and should be implemented in order to enable provision watchers feature in the device service

Original issue here:
edgexfoundry/edgex-go#1101

Resulting from the Device Service Working Group call 11-Feb-19

• The Device struct should contain a map of the properties necessary for connecting to a device. This will be a more flexible structure to connect via any number of protocols rather than overloading a type (Addressable) that is specific to HTTP
• Since the Addressable type will remain for other purposes, the above supporting property should be called ProtocolProperties of type map[string]string
• The ProtocolProperties will be stored as part of the Device document in the database. No sharing is required as in the case of the existing Addressable, thus no requirements for uniqueness.
• Remember to modify the DB storage logic to remove the Addressable mgmt for Devices.

Steve Osselton offered the following additional requirements:
@tsconn23 Need to remember that a device can support multiple protocol types (e.g. modbus-ip/modbus-rtu) so in terms of modelling a device I think we have (using Thrift as a definition):

struct ProtocolProperty
{
  1: string name;
  2: string value;
}
typedef list <ProtocolProperty> ProtocolProperties;

struct Protocol
{
  1: string type;
  2: ProtocolProperties properties;
}
typedef list <Protocol> Protocols;

struct Device
{
  1: Protocols protocols;
}

Pending merge of PR #35.

The Add() function of event.go within the core-data client should check the incoming Context for a ContentType value. If a value is not found, then ContentTypeJson should be the default. However if ContentTypeCbor is found, then the client should marshal the incoming event as CBOR instead of JSON. This should still allow the client to pass a byte array to the clients.PostRequest() method.

The current candidate for accomplishing CBOR marshalling is ugorji. This library has no dependencies other than the Go std lib.

Add a version file for tagging support. Initial rev @ v0.0.1

The purpose of this change is to provide a new mechanism for device services to autonomously generate events, and to be able to take advantage of devices which can push new readings rather than relying on polling.

There is a FloatEncoding field in both PropertyValue and ValueDescriptor, and the value is limited to Base64 and eNotation. Thus, we should add constants for them.

There is no addressable in Device data model, so DevicesForAddressable and DevicesForAddressableByName functions should be removed

From the discussion in edgexfoundry/device-sdk-go#150, the Parameter field in ResourceOperation is useless now.

In today's working group meeting, we decide to use duration string for Frequency of AutoEvent

https://golang.org/pkg/time/#example_ParseDuration

See edgexfoundry/edgex-go#1194 for the full context.

Meanwhile the issue here is that this method is uncallable since it isn't on the ReadingClient interface. Based on the issue above, it appears this method and any related tests can be removed.

modify
Protocols map[string]map[string]string
to

Protocols      map[string]ProtocolProperties
type ProtocolProperties map[string]string

Changes are required to facilitate the functionality described in the following edgex-go issue
edgexfoundry/edgex-go#1244

This issue will give us the opportunity to introduce functional "request types" into our contracts -- that is, request models that are specific to a particular API endpoint rather than general models which are utilized in various ways for various endpoints. Our current contract models were originally state models within edgex-go. They have been refactored out into their own module now, which is good, but we need to go further so that requests become expressions of intent and support encapsulated validation (per #66).

An example of the above, albeit for a different use case, can be found in the following feature branch.
https://github.com/tsconn23/go-mod-core-contracts/tree/marshal_bug/requests/provision_watcher

Utilizing the package naming best practices, I would suggest creating a requests package at the same level as models and clients. Then underneath that, create a package structure like the following

states

• admin
• operating

The admin and operating packages above should each contain an update.go wherein the respective UpdateRequest is defined. The request's sole member would be either models/OperatingState or models/AdminState so that JSON marshaling will create a request that looks like this:
{"adminState":"unlocked"}
{"operatingState":"disabled"}

There should also be tests, as in the example branch, but we'll need to see how the timing works out between this issue and #66 which is implementing a new comprehensive model validation scheme. In effect, the work of #66 (which you can see in the feature branch linked in the comments there) will allow these new request types to call Validate() on their respective AdminState or OperatingState members. If the work on this effort completes first, we may just write the tests as part of #66.

I recently reported an issue against device-sdk-go that concerned dumping entire objects in non-debug log messages.

The offending log message was actually including the entire state of a Device object, which is enormous! This got me to thinking whether it's actually a good idea for us to be embedding an entire DeviceService in every device as opposed to DeviceService.Name (or ID)?
Note, this is related to issue #27 which proposes removing DeviceProfile from Device.

The EventClient needs to be extended to allow an event to be marked as "pushed" using its checksum. The new underlying API endpoint is defined here. This endpoint will be used by app-functions-sdk-go and export-distro (until it's deprecation) after an event has been handled.

This is part of the CBOR workstream.

As discussed at length over the last month in the Core WG calls and via email, we need to implement validation of the request models currently used in core-contracts. The settled on approach entails

• Creating a Validator interface to which any struct not aliased to a primitive type can implement
• Common validation logic will utilize reflection to validate all complex struct types implementing this interface which are children of the root model
• Implementers of the interface will mark themselves as having been already validated the first time their Validate() method is called.

For later, there will be a more serious discussion around refactoring of request types to be more specific and contextual to the REST endpoint they address. For the Edinburgh release, hat extensive of refactoring, which will have to be integrated in several repos, isn't feasible.

We have found that while the Event model may encode to CBOR correctly as is, it will not decode properly due to the absence of field name in the encode tags. Need to add these, similar to how we've done with JSON.

We need to remove the Schedule and ScheduleEvent models from this project as their supported endpoint is deprecated. Remove associated tests as well.

When IntervalAction is marshaled to JSON, Path parameter is lost
https://github.com/edgexfoundry/go-mod-core-contracts/blob/master/models/interval_action.go#L43

Do a module-wide audit of GoDoc conformant comments in order to provide updated online documentation.

There is currently no validation of the Type field of a ValueDescriptor.
We should add validation logic which enforces that type can only be one of the following values:

• bool
• string
• int8|int16|int32|int64
• uint8|uint16|uint32|uint64
• float32|float64
• binary

The initial population of this repo should contain code from the /pkg folder of edgex-go. This module will eventually replace said folder.

When unmarshaling the DeviceProfile, the Description is not currently being set from the Alias type.

Root cause of edgexfoundry/edgex-go#1320

Source issue edgexfoundry/device-sdk-go#269

In order to accomplish this, the following validation requirement that a reading must have a Device name assigned has to be relaxed

Following from the above, there's already a PR in edgex-go which will assign the Device name from the Event to each reading before persisting to the database.
edgexfoundry/edgex-go#1324

Once PR #1324 is merged and this issue is addressed, then device-sdk-go edgexfoundry/device-sdk-go#271 should be ready for integration.

Version 1.1.4 of github.com/ugorji/go was recently released.

We need to update go.mod to pin to this version of the module.

Per the Device Services Working Group call 1-Apr-2019 the following properties on the DeviceProfile struct should be renamed.

• Resources --> DeviceCommands
• Commands --> CoreCommands

After this change integration PRs will be needed in

• edgex-go
• device-sdk-go
• blackbox-testing

The type of frequency ought to be uint32 rather than int32
https://github.com/edgexfoundry/go-mod-core-contracts/blob/master/models/autoevent.go#L20

Also, we had better add some document into autoevent.go

The BaseObject type embedded in 19 of the models currently needs both consistent handling and a rename. If it is to be kept, then:

• it needs to be introduced into all models that have the associated fields in their signature -- Origin,Created,Modified. These are all defined as int64 and represent timestamps.
• the name BaseObject needs to be replaced with something more meaningful to its presence. As described above, all of the fields are timestamps. Therefore I suggest we call the type Timestamps.

The name BaseObject is also objectionable because it suggests inheritance over composition. Go's embedded types are not a form of inheritance, they are instead composition. FWIW, this name is a holdover from the original Java --> Go conversion.

As an aside, it's worth noting here that I have previously been advocating the removal of BaseObject and instead placing its properties directly inline into each type that currently uses it. I'm rethinking that position due to the following resources:

• The discussion in this thread
• From "The Go Programming Language" book -- pp. 104-107 (Struct Embedding and Anonymous Fields), pp.161-164 (Composing Types by Struct Embedding)

Besides the name, I didn't like that the BaseObject type was merely a property bag. It didn't define any particular behavior or responsibility. To me that was a code smell. However if you look at the book examples above, this is exactly the kind of types they use to compose the ColoredPoint scenario. In some ways, I am still uncomfortable with a hierarchical organization of types for very little semantic meaning. But if we rename BaseObject to something more indicative of what its role is (like Timestamps) then I think it's more palatable to retain.

When the contract models fail to validate their internal state, they should return a specific error type to allow for more specific error handling within the API controller functions. For example, when performing unmarshaling within a controller, a type check on the returned error will allow us to be more explicit about whether we should return a 400 Bad Request in the case of a model's invalid state versus some unexpected condition. The latter would result in a 500. response.

Add a go fmt ./... step to the make test target in the makefile. Run it, and then check in the makefile addition with the results.

As per device services WG discussion on 3/25, the use of frequency in AutoEvents as they stand is inconvenient.
Replace Frequency with an Interval field, with string type.
The format of this field is to be an unsigned integer followed by a unit which may be "ms", "s", "m" or "h" representing milliseconds, seconds, minutes or hours. Eg, "100ms", "24h"

For example, the following request to core-metadata to add a device fails because there is no OperatingState given for the device service:

curl -d '{"profile":{"name":"Example Sensor"},"service":{"name":"device-template"},"lastConnected":0,"lastReported":0,"operatingState":"ENABLED","adminState":"UNLOCKED","protocols":{"Other":{"Port":"502","Address":"someSensorEndpoint"}},"name":"TemplateDevice","description":"An Example Template Device","origin":0}' -X POST 0:48081/api/v1/device

This ones on me... as I suggested the wrong struct in my review comment for PR #35 where I said:

So here's the deal, having MediaType in the ProfileProperty is required, as that's part of the DeviceResource struct, however if a client of Core Data wants to understand the contents of a specific Reading, the MediaType should also be present in the value descriptor.

ProfileProperty is a struct which contains PropertyValue and Units. The former describes the type of the device resource and latter describes the units used to label it. MediaType should actually be part of PropertyValue, as it's another attribute which describes the value of the device resource. The device service SDKs use the PropertyValue to create a ValueDescriptor instance.

Note that the current https://github.com/edgexfoundry/go-mod-core-contracts/tree/master/clients/logging directory contains files that redefine their package as logger.

Example:

I have already fixed this locally and based on work to integrate into the other applications which depend on the logging, it appears the least invasive change to sync the names is to rename the logging directory to logger. This means imports in the consuming applications will need to change from

github.com/edgexfoundry/go-mod-core-contracts/clients/logging

to

github.com/edgexfoundry/go-mod-core-contracts/clients/logger

This is because applications are already using the logger name in most cases.
var LoggingClient logger.LoggingClient