-
Notifications
You must be signed in to change notification settings - Fork 310
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
Support payload normalization #5730
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.
I like the direction in which this is going.
Now that the uplink payload formatter parameter encapsulates two functions at once, should we consider increasing the character limit, or do we keep the one we have so far ?
Let's not do that until we run into problems. I think we're already quite generous in the Device Repository with 64KB. Inline codecs in The Things Stack have a lower limit, but since we support detecting whether the payload is already normalized, what is really needed is that codec developers just return normalized payload going forward. |
1a770b0
to
94c35a4
Compare
@adriansmares this is ready for another full pass You can skip the first commit; all lint warning removals are squashed in there, no functional changes otherwise |
9bab7cd
to
3f1e623
Compare
3f1e623
to
25dc6ac
Compare
This comment was marked as resolved.
This comment was marked as resolved.
25dc6ac
to
6db9aff
Compare
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.
Webhook template store schema (the YAML files that describe the templates) require an update with the new message type too. See
lorawan-stack/pkg/applicationserver/io/web/templates.go
Lines 225 to 236 in 6db9aff
type webhookTemplatePaths struct { | |
UplinkMessage *string `yaml:"uplink-message,omitempty"` | |
JoinAccept *string `yaml:"join-accept,omitempty"` | |
DownlinkAck *string `yaml:"downlink-ack,omitempty"` | |
DownlinkNack *string `yaml:"downlink-nack,omitempty"` | |
DownlinkSent *string `yaml:"downlink-sent,omitempty"` | |
DownlinkFailed *string `yaml:"downlink-failed,omitempty"` | |
DownlinkQueued *string `yaml:"downlink-queued,omitempty"` | |
DownlinkQueueInvalidated *string `yaml:"downlink-queue-invalidated,omitempty"` | |
LocationSolved *string `yaml:"location-solved,omitempty"` | |
ServiceData *string `yaml:"service-data,omitempty"` | |
} |
5fb6443
to
48542f1
Compare
Summary
Closes #5429
References TheThingsNetwork/lorawan-devices#395
Changes
normalized_payload
to upstream messagesTesting
Unit testing and local integration testing
Regressions
None expected
Notes for Reviewers
Skip the first commit; all lint warning removals are squashed in there
This includes some commits from #5727 to show non-LoRa data rates in the event preview
@kschiffer please review
console:
commit onlyWe can probably do improve, but this is already pretty cool:
Checklist
README.md
for the chosen target branch.CHANGELOG.md
.CONTRIBUTING.md
, there are no fixup commits left.