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

Providing a Common MQTT Mapper #128

Closed
wants to merge 4 commits into from
Closed

Conversation

fuchendou
Copy link

@fuchendou fuchendou commented Sep 17, 2024

This PR introduces a universal MQTT Mapper designed to handle different serialization formats (JSON, YAML, and XML) during MQTT transmissions. The goal is to enhance flexibility and scalability for device communication.

Main Changes:

  1. devicetype.go:

Improved ConfigData and VisitorConfigData structs to handle data before and after MQTT parsing.

  1. driver.go:

Implemented methods for the ConfigData struct, including logic for serialization and parsing.

This PR fixes the issue: PR #链接

Signed-off-by: Prometheus-collab <[email protected]>
@kubeedge-bot
Copy link
Collaborator

Welcome @fuchendou! It looks like this is your first PR to kubeedge/mappers-go 🎉

@kubeedge-bot kubeedge-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Sep 17, 2024
@kubeedge-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign chendave after the PR has been reviewed.
You can assign the PR to them by writing /assign @chendave in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: Prometheus-collab <[email protected]>
Comment on lines 41 to 47
DataType string `json:"dataType"`

ClientID string `json:"clientID"` // MQTT Client ID
DeviceInfo string `json:"deviceInfo"` // Device information, such as device identification or other important information.
OperationInfo OperationInfoType `json:"operationInfo"` // Operation information, such as adding, deleting, modifying and so on.
SerializedFormat SerializedFormatType `json:"fileType"` // Supported formats: json, xml and yaml.
ParsedMessage map[string]interface{} `json:"parsedMessage"` // The parsed message
Copy link
Collaborator

Choose a reason for hiding this comment

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

Because mapper is a device management plug-in in kubeedge, it needs to complete the function of communicating with the actual edge IoT device and obtaining device data. According to your definition here, can mapper obtain the information and data of the actual mqtt device by subscribing to this ClientID?

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your feedback! I have enriched the MQTT configuration fields in ConfigData.

Comment on lines 57 to 71
func (client *CustomizedClient) SafeSubscribe(topic string) (interface{}, error) {
client.deviceMutex.Lock()
defer client.deviceMutex.Unlock()

if client.MessageQueue == nil {
return nil, errors.New("message queue is not initialized")
}

// Call the Subscribe method of the message queue
msg, err := client.MessageQueue.Subscribe(topic)
if err != nil {
return nil, fmt.Errorf("failed to subscribe to topic: %v", err)
}
return msg, nil
}
Copy link
Collaborator

@wbc6080 wbc6080 Sep 19, 2024

Choose a reason for hiding this comment

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

In the design of mapper, the driver file is mainly used to complete the device driver function, that is, the function of connecting to the actual IoT device and obtaining device data. It is necessary to implement the functions of InitDevice GetDeviceData SetDeviceData and StopDevice according to the framework generated by mapper-framework. It seems that you have implemented the function of obtaining device data by subscribing to the device mqtt id, but you have not named the function to getdevicedata. In this way, the mapper data plane cannot obtain the device data normally and cannot push it to the cloud or user applications. For details, you can refer to the previous mapper implementation, which needs to include the functions of initdevice getdevicedata and more in the driver. https://github.com/kubeedge/mappers-go/blob/main/mappers/kubeedge-v1.15.0/usbcamera-dmi/driver/driver.go#L48
https://github.com/kubeedge/mappers-go/blob/main/mappers/kubeedge-v1.17.0/onvif-mapper/driver/driver.go#L26

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your input! I have added the necessary functions to the code based on your feedback.

Signed-off-by: Prometheus-collab <[email protected]>
Comment on lines 35 to 38
func (c *CustomizedClient) GetDeviceData(visitor *VisitorConfig) (interface{}, error) {

return nil, nil
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

In other words, the mqtt mapper cannot get the value from the device, right?

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your feedback! I have added the necessary functions to the code based on your feedback.

Comment on lines 13 to 14
"github.com/kubeedge/mapper-framework/pkg/common"
"gopkg.in/yaml.v3"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please sort the import, those begin with github.com/kubeedge should be placed in the third part

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your feedback! I have adjusted the order of the imports as you requested.

@@ -0,0 +1,41 @@
module github.com/kubeedge/kubeedge-v1.17.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

The module name is not suitable. We usually use github.com/kubeedge/{mapperName}

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your feedback! I have updated the module name in config.yaml to follow the format, which is github.com/kubeedge/{mapperName}.

@@ -0,0 +1,15 @@
apiVersion: v1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your suggestion! I have added the model and instance files to the appropriate locations as requested.

Signed-off-by: Prometheus-collab <[email protected]>
@fuchendou fuchendou requested a review from wbc6080 September 26, 2024 09:58
@fuchendou fuchendou closed this Sep 29, 2024
@fuchendou fuchendou reopened this Sep 29, 2024
@fuchendou fuchendou closed this Sep 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants