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.

Sign up for GitHub

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 ability for receiver to check if CRC is present in payload #333

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Add ability for receiver to check if CRC is present in payload #333

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
eefa5d2
Update LoRa.h
mcgurk Feb 3, 2020
67fcf5b
Update LoRa.cpp
mcgurk Feb 3, 2020
301fbeb
Update keywords.txt
mcgurk Feb 3, 2020
a679a40
Update LoRa.cpp
mcgurk Feb 3, 2020
c6eafb7
Update LoRa.h
mcgurk Feb 3, 2020
1798643
Update keywords.txt
mcgurk Feb 4, 2020
64d8cc7
Update keywords.txt
mcgurk Feb 25, 2020
f1415cb
Update LoRa.cpp
mcgurk Feb 25, 2020
ae1a552
Update LoRa.h
mcgurk Feb 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions keywords.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ setPreambleLength KEYWORD2
setSyncWord KEYWORD2
enableCrc KEYWORD2
disableCrc KEYWORD2
crcOnPayload KEYWORD2
enableInvertIQ KEYWORD2
disableInvertIQ KEYWORD2

Expand Down
6 changes: 6 additions & 0 deletions src/LoRa.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#define REG_RX_NB_BYTES 0x13
#define REG_PKT_SNR_VALUE 0x19
#define REG_PKT_RSSI_VALUE 0x1a
#define REG_HOP_CHANNEL 0x1c
#define REG_MODEM_CONFIG_1 0x1d
#define REG_MODEM_CONFIG_2 0x1e
#define REG_PREAMBLE_MSB 0x20
Expand Down Expand Up @@ -582,6 +583,11 @@ void LoRaClass::disableCrc()
writeRegister(REG_MODEM_CONFIG_2, readRegister(REG_MODEM_CONFIG_2) & 0xfb);
}

uint8_t LoRaClass::crcOnPayload()
{
return (readRegister(REG_HOP_CHANNEL) >> 6) & 1;
}

void LoRaClass::enableInvertIQ()
{
writeRegister(REG_INVERTIQ, 0x66);
Expand Down
1 change: 1 addition & 0 deletions src/LoRa.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ class LoRaClass : public Stream {
void setSyncWord(int sw);
void enableCrc();
void disableCrc();
uint8_t crcOnPayload();
Copy link
Owner

Choose a reason for hiding this comment

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

I'm thinking this could be renamed to something more consistent with: https://github.com/sandeepmistry/arduino-LoRa/blob/master/src/LoRa.h#L43-L46

Maybe: packetHasCrc()?

Also, for the return type type is bool a better option?

Thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

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

isCrcPresent?

Normally a bool variable may start with is...?

Copy link
Author

@mcgurk mcgurk Feb 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

As far as I know, in Arduino IDE bool is same than uint8_t (unsigned 8bit integer). I have habit to use uint8_t, but I have nothing against bool. bool is just little bit more abstract.

I think too that crcOnPayload is little problematic function name. It is accurate, but it doesn't indicate what it means to reader. Both your suggestions are better than "crcOnPayload".

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @mcgurk, if you can address these minor changes I'll merge this PR and include in the next release I'd like to get out this week. Thanks for this contribution and keen eyed reading of the datasheet.

Copy link
Author

Choose a reason for hiding this comment

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

Hmm.. This is my first pull request ever :D. What I have to do? Edit my branch? I try.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah you just push additional commits to your branch and they will show up here.

void enableInvertIQ();
void disableInvertIQ();

Expand Down