-
Notifications
You must be signed in to change notification settings - Fork 67
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
Provide a Common MQTT Mapper #130
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that this PR can't pass the CI check. Can you try to find the reason?
Thanks to your letter, I observed the returned error, changed the file
permissions and resubmitted the code.
…On Sat, Oct 12, 2024 at 11:06 AM Elias Wang ***@***.***> wrote:
***@***.**** commented on this pull request.
It seems that this PR can't pass the CI check. Can you try to find the
reason?
—
Reply to this email directly, view it on GitHub
<#130 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A62LQCMYZ6HSSYANYB3D2S3Z3CG2FAVCNFSM6AAAAABPBUNBI2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGNRTG4ZDKNBWGM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that there is still a problem with CI. I think it is due to a dependency mismatch caused by the existence of v1alpha2 and v1beta1 mappers in this directory. I suggest placing this mapper in the newly created directory mappers-go/mappers/device-v1beta1/, which should solve the current CI error problem.
mappers/mqtt-mapper/go.mod
Outdated
@@ -0,0 +1,41 @@ | |||
module github.com/kubeedge/kubeedge-v1.17.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name of the package is incorrect. You should set the mapper name to mqtt
instead of kubeedge-v1.17.0
when executing the make generate
command to generate the mapper project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your advice. I will remake a new mapper project using make generate
.
mappers/mqtt-mapper/Dockerfile
Outdated
FROM golang:1.20.10-alpine3.18 AS builder | ||
|
||
WORKDIR /build | ||
|
||
ENV GO111MODULE=on \ | ||
GOPROXY=https://goproxy.cn,direct | ||
|
||
COPY . . | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your advice. I will modify the file structure.
Thanks for the feedback! I'll make changes based on your comments.
…On Tue, Oct 15, 2024 at 3:49 PM Elias Wang ***@***.***> wrote:
***@***.**** commented on this pull request.
It seems that there is still a problem with CI. I think it is due to a
dependency mismatch caused by the existence of v1alpha2 and v1beta1 mappers
in this directory. I suggest placing this mapper in the newly created
directory mappers-go/mappers/device-v1beta1/, which should solve the
current CI error problem.
------------------------------
In mappers/mqtt-mapper/go.mod
<#130 (comment)>:
> @@ -0,0 +1,41 @@
+module github.com/kubeedge/kubeedge-v1.17.0
The name of the package is incorrect. You should set the mapper name to
mqtt instead of kubeedge-v1.17.0 when executing the make generate command
to generate the mapper project.
------------------------------
In mappers/mqtt-mapper/Dockerfile
<#130 (comment)>:
> +FROM golang:1.20.10-alpine3.18 AS builder
+
+WORKDIR /build
+
+ENV GO111MODULE=on \
+ GOPROXY=https://goproxy.cn,direct
+
+COPY . .
+
remove this file
—
Reply to this email directly, view it on GitHub
<#130 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A62LQCOLXFGOY5KLGCWLGKLZ3TCJVAVCNFSM6AAAAABPBUNBI2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGNRYGQ2DMMJXGM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job! Thanks for your contribution!
/lgtm
Thank you for your support!
…On Thu, Oct 17, 2024 at 4:28 PM Elias Wang ***@***.***> wrote:
***@***.**** commented on this pull request.
Good job! Thanks for your contribution!
/lgtm
—
Reply to this email directly, view it on GitHub
<#130 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A62LQCM644L44EKA2FAZRZLZ35YJ3AVCNFSM6AAAAABPBUNBI2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGNZUGUZDAMZRGA>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
Could you please help review my proposal "Add a Proposal about Provide a
Common MQTT Mapper" (PR #5765
<kubeedge/kubeedge#5765>)? The detail
implementation has been successfully tested(PR #130
<#130>), and I am looking
forward to your valuable feedback to proceed further.
Thanks for your time and support!
On Fri, Oct 18, 2024 at 9:12 AM mengxiao zhang ***@***.***>
wrote:
… Thank you for your support!
On Thu, Oct 17, 2024 at 4:28 PM Elias Wang ***@***.***>
wrote:
> ***@***.**** commented on this pull request.
>
> Good job! Thanks for your contribution!
> /lgtm
>
> —
> Reply to this email directly, view it on GitHub
> <#130 (review)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/A62LQCM644L44EKA2FAZRZLZ35YJ3AVCNFSM6AAAAABPBUNBI2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGNZUGUZDAMZRGA>
> .
> You are receiving this because you modified the open/close state.Message
> ID: ***@***.***>
>
|
I have leave some comments that you can refer to. @fuchendou |
Thank you for your comments, I have made the corresponding changes for them
and you can retest.
…On Wed, Oct 23, 2024 at 10:57 AM Elias Wang ***@***.***> wrote:
Could you please help review my proposal "Add a Proposal about Provide a
Common MQTT Mapper" (PR #5765
I have leave some comments that you can refer to. @fuchendou
<https://github.com/fuchendou>
—
Reply to this email directly, view it on GitHub
<#130 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A62LQCNCK7OIR2AVUZU5JJDZ44GCRAVCNFSM6AAAAABPBUNBI2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMZQG4ZDIMZYHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
/lgtm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebase the commits |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can refer to https://kubeedge.io/docs/developer/Supplemental_developer_guide#ensuring-to-squash-similiar-commits to squash commits @fuchendou
Thank you for your tips! I make it.
…On Tue, Oct 29, 2024 at 8:38 PM Elias Wang ***@***.***> wrote:
***@***.**** commented on this pull request.
You can refer to
https://kubeedge.io/docs/developer/Supplemental_developer_guide#ensuring-to-squash-similiar-commits
to squash commits @fuchendou <https://github.com/fuchendou>
—
Reply to this email directly, view it on GitHub
<#130 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A62LQCIDG6F44T25O6DFT3TZ556U3AVCNFSM6AAAAABPBUNBI2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDIMBRG4ZTKMJZGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
/assign @WillardHu @wbc6080
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fisherxu 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 |
Signed-off-by: Prometheus-collab <[email protected]> Signed-off-by: fuchendou <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
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:
devicetype.go:
Improved ConfigData and VisitorConfigData structs to handle data before and after MQTT parsing.
driver.go:
Implemented methods for the ConfigData struct, including logic for serialization and parsing.
This PR fixes the issue: PR #链接(kubeedge/kubeedge#5765)