edgexfoundry / go-mod-core-contracts Goto Github PK
View Code? Open in Web Editor NEWOwner: Core/Support WG
License: Apache License 2.0
Owner: Core/Support WG
License: Apache License 2.0
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
In today's working group meeting, we decide to use duration string for Frequency of AutoEvent
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.
Provision Watcher client is not implemented and should be implemented in order to enable provision watchers feature in the device service
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.
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
go-mod-core-contracts/models/reading.go
Line 126 in b02b9d9
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.
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
Currently when marshaling a DeviceProfile to json or yaml, there are lots of empty fields, which should be great if they are removed.
We need to remove the Schedule and ScheduleEvent models from this project as their supported endpoint is deprecated. Remove associated tests as well.
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.
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.
Original issue edgexfoundry/edgex-go#594
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.
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
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.
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
When unmarshaling the DeviceProfile, the Description is not currently being set from the Alias type.
Root cause of edgexfoundry/edgex-go#1320
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"
(as discussed dev svcs wg 4-8-19).
Remove models/Units.
The Filter
property on the Registration model is being marshaled with the name filter.go
. Please fix both the marshaling and unmarshaling.
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.
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.
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.
modify
Protocols map[string]map[string]string
to
Protocols map[string]ProtocolProperties
type ProtocolProperties map[string]string
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
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
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.
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.
From the discussion in edgexfoundry/device-sdk-go#150, the Parameter field in ResourceOperation is useless now.
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
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.
Like it says in the title -- README!!!!
Add a version file for tagging support. Initial rev @ v0.0.1
There is no addressable in Device data model, so DevicesForAddressable and DevicesForAddressableByName functions should be removed
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.
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.
Per the Device Services Working Group call 1-Apr-2019 the following properties on the DeviceProfile struct should be renamed.
After this change integration PRs will be needed in
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.
The BaseObject
type embedded in 19 of the models currently needs both consistent handling and a rename. If it is to be kept, then:
Origin
,Created
,Modified
. These are all defined as int64
and represent timestamps.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:
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.
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.
Original issue here:
edgexfoundry/edgex-go#1101
Resulting from the Device Service Working Group call 11-Feb-19
ProtocolProperties
of type map[string]string
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.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;
}
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.
Do a module-wide audit of GoDoc conformant comments in order to provide updated online documentation.
Very clearly there is a cut-and-paste error here. This was introduced as part of the model validation PR #66.
go-mod-core-contracts/models/reading.go
Line 112 in b003e27
You can see the Id
property on the Reading is erroneously being assigned to Value
as opposed to the Value
property during unmarshaling.
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:
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:
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.
The initial population of this repo should contain code from the /pkg folder of edgex-go. This module will eventually replace said folder.
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
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.
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.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.