-
Notifications
You must be signed in to change notification settings - Fork 230
sx127x: add functions used for FSK radio communication #823
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
Conversation
|
Review requested please! 😸 |
HattoriHanzo031
left a comment
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.
Just some small, non-functional adjustments for clarity/consistency.
| d.spiRxBuf = append(d.spiRxBuf, 0) | ||
| d.spi.Tx(nil, d.spiRxBuf) | ||
| d.controller.SetNss(true) | ||
| if d.controller != nil { |
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 there a use-case of using the driver without the controller? If yes, maybe we could in New set the default nopControler that does nothing to avoid this checks.
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 like to leave that question for a future PR.
| controller RadioController // to manage interrupts with the radio | ||
| deepSleep bool // Internal Sleep state | ||
| deviceType int // sx1261,sx1262,sx1268 (defaults sx1261) | ||
| deviceType int // sx1272, sx1273, sx1276, sx1279 (defaults sx1276) |
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.
deviceType is actually not used anywhere
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 was an oversight on my part, I think. I have added SetDeviceType() to make consistent with sx126x.
f68682a to
5ffd6fa
Compare
|
Thanks for the feedback @HattoriHanzo031 I have made a few updates. I also added a smoketest for the |
HattoriHanzo031
left a comment
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 good to me (after unused constants are removed)
Signed-off-by: deadprogram <ron@hybridgroup.com>
5ffd6fa to
37ca66b
Compare
This PR adds a few missing functions to the
sx127xthat are used for FSK radio communication. It also cleans up a few existing functions to clarify the implementation. Also tested that Lora still works!