-
Notifications
You must be signed in to change notification settings - Fork 48
DOCS-3457: Document LoRaWAN support in Viam #4376
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for viam-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
looks really good!! left some suggestions but feel free to ignore any you don't agree with
{{< tabs >}} | ||
{{% tab name="Python" %}} | ||
|
||
```python |
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 would expect that the calibration commands would be easiest to use from within the app UI (since the sensor needs to be in specific solutions before sending the downlinks). Might just want to include that its also an options to do these commands from the user interface
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.
Great point, I don't think we point to DoCommand in the UI very much but this is a perfect use case to start.
… users can omit keys and session keys from device-specific models
Before I dive into this, did we make a conscious decision to give this an official docs page, like, as a model page? In the past we've said "If it's a module, the docs live in the readme. If it's a built-in model, the docs live in the docs." BUT I know there was also talk at some point of this one being a bigger deal than some, so wanting to have more docs around it. I'm thinking maybe this should be a how-to instead of a reference model page though 🤔 |
@JessamyT I discussed some options for my approach with Naomi and some other stakeholders a week or two ago. I believe we specifically discussed the cartographer service page, the mlmodel service page, and alert on machine telemetry. I'm increasingly leaning towards pulling this out of reference, though, because there is a fair amount of conceptual and how-to content that you inevitably have to cover before you can talk about LoRaWAN. Mostly because our reference pages are... difficult to find, to say the least. They're almost a secret! Any suggestions for where we could put this how-to? Maybe under Connect devices? But it doesn't feel as introductory as most of that material. I'd appreciate your thoughts! |
- **uplink** transmits from nodes to gateways | ||
- **downlink** transmits from gateways to nodes | ||
|
||
 |
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.
[opt] Gets a smidge big on very wide screens; consider setting the width and adding the imgzoom class
But maybe you're getting a pretty visual from Cristina anyway?
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.
That's the plan. Let me know if you have any suggestions for the visual before I send in my request; I'd rather not bother Cristina more than once!
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.
https://viam.atlassian.net/browse/BD-386 submitted to create a branding-themed asset. ETA: next week.
Ah okay! Hmm I suppose if it was in tutorials it wouldn't be any less hidden, and it's not project-based so that wouldn't make sense (it isn't actually a tutorial). So maybe this is actually as good a place as any, BUT we should un-hide components from reference. We've talked about doing this anyway and maybe we should just...actually do that? In any case, if we send a link to a customer, or link to this from a blog post or someone finds it in search or ask ai, and you link to it from the module readme, it'll be available, which I guess is the important thing. |
Un-hiding components in this PR because otherwise you can't navigate to this page without the link. |
Co-authored-by: Jessamy Taylor <[email protected]>
Technically you can--you just have to go component APIs -> sensor API -> click the link in the first sentence -> model page 😈 Easy peasy XD |
Truly a set of instructions worthy of the smiling imp emoji! |
Incorporating some of the table bits here: viam-modules/lorawan#68 Removing some of that data from this doc to reduce redundancy. |
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.
Some minor comments. Mostly I think I wonder if frame port and decoder script really need to have their own sections rather than being explained in the instructions or in the attribute information
…ions, move calibration instructions into this doc from readme
@npentrel @JessamyT when you get the chance (i know this is a busy week with other things), give this another look and let me know if you see any other issues. I should have addressed all of your feedback. I'd be happy to merge this sooner and follow-up with a PR to add the updated diagram next week, but I'll defer to your stance on diagrams. |
|
||
Use the calibration processes below to calibrate your sensor with these commands: | ||
|
||
#### Calibrate the pH probe |
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.
Is this a common enough thing that we want to cover it here? Leaving that up to you but it feels odd to me
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 is a requirement to properly use this sensor model, which is one of our small number of officially supported sensors. And the pattern is generalizable across all other sensors that require calibration. So I'm leaning 'include' unless someone feels really strongly otherwise. I understand how it might feel odd but I think it's important to draw attention to this so folks don't wind up with a lot of uncalibrated sensors with unreliable readings.
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.
Seems good to merge % nits. Though I am also a bit confused where the pH docs came from and whether they should be here. But I'll leave that to you to figure out and approve so I'm not blocking while on leave
Co-authored-by: Naomi Pentrel <[email protected]>
@JessamyT if you could give this another look before I merge, i would appreciate it! |
Adds documentation for the lorawan module.
Preview here: https://deploy-preview-4376--viam-docs.netlify.app/operate/reference/components/sensor/lorawan/