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

Add customized mapper #105

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add customized mapper #105

wants to merge 3 commits into from

Conversation

QiQi-OvO
Copy link
Contributor

@QiQi-OvO QiQi-OvO commented Jul 13, 2023

What type of PR is this?
/kind feature

What this PR does / why we need it:
Now, we can use make dmitemplate to get a dmi template. And provided a example to help developers start their work.

Which issue(s) this PR fixes:
Implemented one feature in kubeedge/kubeedge#4744

Special notes for your reviewer:
You can find more details in exmaple.

Signed-off-by: QiQi-OvO <[email protected]>

add customized mapper

Signed-off-by: QiQi-OvO <[email protected]>

fix some bugs

Signed-off-by: QiQi-OvO <[email protected]>
@@ -48,6 +48,7 @@ func main() {
SockPath: c.GrpcServer.SocketPath,
Protocol: common.ProtocolModbus,
},
device.NewDevPanel(),
Copy link
Contributor

Choose a reason for hiding this comment

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

There may be a mistake?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The parameters of the NewServer function have been changed, because of this change can simplify the code and enable compatibility with more devices without importing corresponding device packages.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I missed it earlier. This is a nice change!

Choose a reason for hiding this comment

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

I tried to reproduce this virtualdevice-dmi example, but the following problem occurred
I0831 21:38:19.609843 659379 device.go:114] visitorConfig: {ProtocolName:virtualProtocol VisitorConfigData:{DataType:}}
I0831 21:38:19.609916 659379 device.go:150] random-instance-01 twin readonly property: random-float
E0831 21:38:20.610317 659379 twindata.go:59] twindata random-float unmarshal failed, err: get device data failed: unrecognized data type

Copy link
Contributor Author

@QiQi-OvO QiQi-OvO Sep 1, 2023

Choose a reason for hiding this comment

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

This seems to be an issue with parsing CRD, are you using mappers/virtualdevice-dmi/resource/random-device-instance.yaml and mappers/virtualdevice-dmi/resource/random-device-model.yaml from PR?
And there is a bug in the DMI code of the current EdgeCore that cannot resolve the customized field. Please refer to this PR.
If you want to know what went wrong during the parsing process, you can unmarshal the value of the visitor to the map[string]interface{} and print it.

@@ -176,6 +178,19 @@ func (s *Server) GetDevice(ctx context.Context, request *dmiapi.GetDeviceRequest
}
res.Device.Status.Twins = twins
res.Device.Status.State = common.DEVSTOK
case common.ProtocolCustomized:
deviceValue := reflect.ValueOf(device)
//d := device.(*customizedDriver.CustomizedDev)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To make this a universal framework. I plan to delete all switch/case structures and not import the corresponding device's packages, and subsequently obtain the corresponding fields through reflection. I will modify it later and hope you can continue to provide valuable feedback. Thanks! :)

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, but I mean //d := device.(*customizedDriver.CustomizedDev) this unused code, Is it possible to remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@kubeedge-bot kubeedge-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 14, 2024
@kubeedge-bot
Copy link
Collaborator

@QiQi-OvO: PR needs rebase.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants