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 dragino sensor #25

Merged
merged 11 commits into from
Feb 14, 2025
Merged

add dragino sensor #25

merged 11 commits into from
Feb 14, 2025

Conversation

JohnN193
Copy link
Collaborator

@JohnN193 JohnN193 commented Feb 13, 2025

initial dragino sensor.

big changes

  1. add helpers for making Nodes
  2. Allow for gateways to be implicit dependencies(without breaking if made explicit instead)
  3. add the dragino sensor with the decoder file included(via http)

also tweaks the gomod/adds some nolints to prevent the linter from breaking everything

Comment on lines +43 to +55
func (conf *Config) getNodeConfig(decoderFilePath string) node.Config {
return node.Config{
JoinType: conf.JoinType,
DecoderPath: decoderFilePath,
Interval: conf.Interval,
DevEUI: conf.DevEUI,
AppKey: conf.AppKey,
AppSKey: conf.AppSKey,
NwkSKey: conf.NwkSKey,
DevAddr: conf.DevAddr,
Gateways: conf.Gateways,
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Basically we have other sensors create their own node.Config, then we can reuse all of the validation used in the node implementation

resource.Named
logger logging.Logger

node node.Node
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the dragino LHT65N wraps a node implementation, so we can easily reuse all of the node functionality

@@ -0,0 +1,36 @@
package draginolht65n
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

pretty much everything is already covered by node_test.go. Once we add more dragino specific fields we will probably add more to here.

@@ -18,26 +16,28 @@ var Model = resource.NewModel("viam", "lorawan", "node")

// Error variables for validation.
var (
errDecoderPathRequired = errors.New("decoder path is required")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

exported these so they could be used in other sensors

meta.json Outdated
Comment on lines 24 to 25
"short_description": "A sensor compoenent for a dragino LHT65N LoRaWAN node."
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"short_description": "A sensor compoenent for a dragino LHT65N LoRaWAN node."
}
"short_description": "A sensor component for a dragino LHT65N LoRaWAN node."
}

And the short_description above too.

@@ -0,0 +1,332 @@
function decodeUplink(input) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be better to get it from the github URL in case they make an update? but this is prob fine

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added a decoder from url option! leaving the embed implementation in case someone needs to use it for some reason(no network? idk)


// WriteDecoderFile writes an embeded decoderFile into the data folder of the module.
func WriteDecoderFile(decoderFilename string, decoderFile embed.FS) (string, error) {
// Create or open the file used to save device data across restarts.
Copy link
Collaborator

Choose a reason for hiding this comment

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

delete or edit comment

if err != nil {
return nil, err
}
n := &LHT65N{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of wrapping the node object I think you could just create a node.Node here, then you could reuse the node's decoderPath field and the Readings/close functions. Just something to consider not 100% sure it would work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried that initially but ran into some issues with reconfigure. I can't reuse node.Node because the reconfigure function is still using a resource.Config that looks for a node.Config with a decoderPath set. Wrapping the object + adding the helpers lets me get around the issue.

n := &LHT65N{
Named: conf.ResourceName().AsNamed(),
logger: logger,
// This returns the name given to the resource by user
Copy link
Collaborator

Choose a reason for hiding this comment

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

delete or edit comment


nodeCfg := cfg.getNodeConfig(n.decoderPath)
if len(deps) == 0 {
n.logger.Info("yo no deps")
Copy link
Collaborator

Choose a reason for hiding this comment

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

remember to change log before merge

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah actually meant to delete this one, was using it to debug something

const decoderFilename = "LHT65NChirpstack4decoder.js"

// Model represents a dragino-LHT65N lorawan node model.
var Model = resource.NewModel("viam", "lorawan", "dragino-LHT65N")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's make a model Family since we're going to be using the "viam", "lorawan" module ID a lot of tims

Copy link
Collaborator

@oliviamiller oliviamiller left a comment

Choose a reason for hiding this comment

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

few nits/questions but lgtm!

conf resource.Config,
logger logging.Logger,
) (sensor.Sensor, error) {
httpClient := &http.Client{
Copy link
Collaborator

Choose a reason for hiding this comment

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

if this is the only time we use the httpClient could we just create it in the WriteDecoderFileFromURL instead of here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have to create the httpClient outside of the function for the mock in testing to work :(


// CheckCaptureFrequency check the resource's capture frequency
// and reports to the user whether a safe value has been configured.
func CheckCaptureFrequency(c resource.Config, interval float64, logger logging.Logger) (bool, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think the bool return value is ever used, could we get rid of it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can't remember the reason why I added it and I didn't write it down, so yeah lets delete it lol

payload = append(payload, netIDLE[:]...)
//nolint:all
Copy link
Collaborator

Choose a reason for hiding this comment

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

tysm for fixing

@JohnN193 JohnN193 merged commit 68a6816 into main Feb 14, 2025
2 checks passed
@JohnN193 JohnN193 deleted the addDragino branch February 14, 2025 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants