Add Mapper SDK #70
Add Mapper SDK #70
Conversation
kubeedge-bot
added
the
kind/feature
|
Welcome @QiQi-OvO! It looks like this is your first PR to kubeedge/mappers-go 🎉 |
kubeedge-bot
added
the
size/XXL
|
hello, Qi @QiQi-OvO . your commit should be provided with sign-off information. you can try to re-commit with this command |
Signed-off-by: QiQi-OvO <2763359332@qq.com>
Signed-off-by: caixindi <1435516315@qq.com>
|
@RyanZhaoXB: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
| "github.com/kubeedge/mappers-go/mapper-sdk-go/internal/instancepool" | ||
| "github.com/kubeedge/mappers-go/mapper-sdk-go/internal/mqttadapter" | ||
| "github.com/kubeedge/mappers-go/mapper-sdk-go/pkg/di" | ||
| "k8s.io/klog/v2" |
There was a problem hiding this comment.
golang的import引用有一定规范，一般是按包的稳定程度，先引用golang系统提供包，再引用第三方提供包，最后引用本项目自己的包，虽然不是必须，但建议遵守。如本文件中的引用顺序可修改为：
import (
"context"
"net/url"
"sync"
"k8s.io/klog/v2"
"github.com/kubeedge/mappers-go/mapper-sdk-go/internal/common"
"github.com/kubeedge/mappers-go/mapper-sdk-go/internal/configmap"
"github.com/kubeedge/mappers-go/mapper-sdk-go/internal/controller"
"github.com/kubeedge/mappers-go/mapper-sdk-go/internal/httpadapter/requests"
"github.com/kubeedge/mappers-go/mapper-sdk-go/internal/instancepool"
"github.com/kubeedge/mappers-go/mapper-sdk-go/internal/mqttadapter"
"github.com/kubeedge/mappers-go/mapper-sdk-go/pkg/di"
)
其他go代码可照此修改，不再单独指出
| // Check if the device ID added by the user is duplicated | ||
| for k := range deviceInstances { | ||
| if k == instanceID { | ||
| klog.Error("The deviceInstance name used in the uploaded file already exists in mapperService") |
There was a problem hiding this comment.
instance信息如name或id可以在log中体现一下，方便调试定位，下同
| if _, ok := protocols[deviceInstance.ProtocolName]; !ok { | ||
| if addDeviceRequest.Protocol != nil { | ||
| i := 0 | ||
| for i = 0; i < len(addDeviceRequest.Protocol); i++ { |
There was a problem hiding this comment.
这类判断元素是否存在的逻辑，都改成for range + isFind判断的方式会不会更简洁一点。另外如果有多个类似逻辑，不知道能否抽象成一个函数，如果不适合抽象就不用改了
| driverName := common.DriverPrefix + deviceInstance.ID + visitorV.PropertyName | ||
| connectInfo[driverName] = &configmap.ConnectInfo{ | ||
| ProtocolCommonConfig: deviceInstance.PProtocol.ProtocolCommonConfig, | ||
| VisitorConfig: tempVisitorV.VisitorConfig, |
|
|
||
| // AddDevice internal callback function | ||
| func AddDevice(addDeviceRequest requests.AddDeviceRequest, dic *di.Container) (kind common.ErrKind) { | ||
| instanceID := addDeviceRequest.DeviceInstance.ID |
There was a problem hiding this comment.
建议改成instanceID := deviceInstance .ID，后面144、183、184行可直接引用instanceID，前后统一，降低代码阅读难度
| cancelFunc() | ||
| modelNameDeleted := deviceInstances[instanceID].Model | ||
| protocolNameDeleted := deviceInstances[instanceID].ProtocolName | ||
| if _, ok := deviceInstances[instanceID]; ok { |
There was a problem hiding this comment.
这里判断deviceInstance是否存在，应该放在198行之前做比较好吧
| delete(stopFunctions, instanceID) | ||
| klog.V(1).Infof("Remove %s successful\n", instanceID) | ||
| } else { | ||
| klog.V(1).Infof("Remove %s failed,there is no such instanceId\n", instanceID) |
| func ReadDeviceData(deviceID string, propertyName string, dic *di.Container) (response string, kind common.ErrKind) { | ||
| deviceInstance := instancepool.DeviceInstancesNameFrom(dic.Get) | ||
| if _, ok := deviceInstance[deviceID]; !ok { | ||
| kind := common.KindEntityDoesNotExist |
There was a problem hiding this comment.
kind已经作为显式命名的返回值，这里是不是可以不用重新声明一个kind，或者直接return common.KindEntityDoesNotExist
| protocolDriver := instancepool.ProtocolDriverNameFrom(dic.Get) | ||
| response, err := controller.GetDeviceData(deviceID, deviceInstance[deviceID].Twins[index], protocolDriver, mapMutex[deviceID], dic) | ||
| if err != nil { | ||
| kind = common.KindServerError |
|
@RyanZhaoXB: changing LGTM is restricted to collaborators In response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Signed-off-by: QiQi-OvO <2763359332@qq.com>
|
@RyanZhaoXB: changing LGTM is restricted to collaborators In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@fisherxu please help to review this pr. thanks. |
Signed-off-by: QiQi-OvO <2763359332@qq.com>
Signed-off-by: QiQi-OvO <2763359332@qq.com>
README.md
Outdated
|
|
||
| ### 2 mapper-go-sdk | ||
|
|
||
| SDK provide RESTful and you can connect your device fastly. |
There was a problem hiding this comment.
What's meaning of SDK provide RESTful? It's better to introduce how the the mapper SDK work here.
README.md
Outdated
| @@ -1,2 +1,11 @@ | |||
| # mappers-go | |||
| KubeEdge Device Mappers written in go, it works for KubeEdge 1.4 version and later. | |||
|
|
|||
| ## Construction method | |||
| ### 1 mappers | |||
There was a problem hiding this comment.
What's meaning of Construction method here? Seems there's no content under mappers title.
It's better not to use the number like 1 in markdown
docs/MapperSDKDesign.md
Outdated
| @@ -0,0 +1,78 @@ | |||
| # Mapper-sdk-go Design | |||
There was a problem hiding this comment.
The file name should be mapper-sdk-design.md
docs/UserGuideofMapperSDK.md
Outdated
| @@ -0,0 +1,95 @@ | |||
| # User Guide of Customized Mapper SDK in KubeEdge | |||
mapper-sdk-go/README.md
Outdated
| @@ -0,0 +1,209 @@ | |||
|   [](https://gitee.com/ascend/mappers-go-sample/blob/mapper-go-sdk/LICENSE) | |||
| @@ -0,0 +1,13 @@ | |||
| package main | |||
There was a problem hiding this comment.
delete the two binaries above this file.
| @@ -0,0 +1,6 @@ | |||
| FROM alpine:3.14 | |||
| RUN mkdir -p kubeedge | |||
| COPY ./bin/Template kubeedge/bin/ | |||
There was a problem hiding this comment.
Should include make in all dockerfiles, users can make the image from code directly
There was a problem hiding this comment.
Thank you for your valuable advice, I am trying to correct other mistakes : ).
For this Dockerfile,the corresponding operation has been given in README, make mapper Template package can build the code and copy the compiled file to the image. Mapper and mapper-sdk-go both follow this rule, so I think it is reasonable to follow the previous make rules and Dockerfile.
QiQi-OvO
force-pushed
the
mapper-sdk-go
branch
2 times, most recently
from
bb97b36
to
2eac86e
Compare
Signed-off-by: QiQi-OvO <2763359332@qq.com>
| get device data and device status through these interfaces. | ||
| Mapper-sdk-go supports connect customized devices to kubeedge fastly. If you want to access a device with a new protocol, | ||
| mapper-sdk-go is a good choice. | ||
| You can get more information and details in [mapper-sdk-go](./mapper-sdk-go/) |
There was a problem hiding this comment.
From the description, I'm not quite understand, users can use mapper-go-sdk diectly without any development?
From the name mapper-go-sdk, it's a lib, users can import it to implement the mapper.
Mapper-sdk-go is written based on the basic mapper, mapper-sdk-go is based on mapper? I think mapper-sdk-go should provide some basic libs or framework to build a new mapper. Users can build a new mapper easily with mapper-sdk-go, only need to implement the customized protocol driver.
Need to update the descriptions above
Signed-off-by: QiQi-OvO <2763359332@qq.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fisherxu, RyanZhaoXB The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kubeedge-bot
added
the
approved
What type of PR is this?
/kind feature
What this PR does / why we need it:
mapper SDK helps developers reduce the workload of developing mapper.
Which issue(s) this PR fixes:
Fixes #69
Special notes for your reviewer: