Skip to content

Autobaud, sleep, wake changes #234

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

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

Autobaud, sleep, wake changes #234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
433a221
Fixed LoRa module not waked from sleep by autoBaud sometimes (#231)
hallard Apr 4, 2018
27a76a8
Clean up branch to only contain autobaud/wake code.
jpmeijers Apr 4, 2018
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
58 changes: 47 additions & 11 deletions src/TheThingsNetwork.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ uint8_t receivedPort(const char *s)
return port;
}

TheThingsNetwork::TheThingsNetwork(Stream &modemStream, Stream &debugStream, ttn_fp_t fp, uint8_t sf, uint8_t fsb)
TheThingsNetwork::TheThingsNetwork(SerialType &modemStream, Stream &debugStream, ttn_fp_t fp, uint8_t sf, uint8_t fsb)
Copy link
Contributor

@cimm cimm Jul 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

  1. Random drive by comment but don't forget to update the documentation since the Stream is changing to a SerialType.
  2. Wouldn't it make sense to change the variable name from modemStream to modemSerial or something to avoid confusion?

{
this->debugStream = &debugStream;
this->modemStream = &modemStream;
Expand Down Expand Up @@ -387,6 +387,12 @@ size_t TheThingsNetwork::readResponse(uint8_t prefixTable, uint8_t indexTable, u
return readLine(buffer, size);
}

size_t TheThingsNetwork::checkModuleAvailable()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Having a function like this is a good idea. I would however like to change it to check the version of the RNxxxx module and return a value from an enum stating which hardware and firmware version it is.

We can then use the result to check before we set the frequency plan, and prevent errors when setting the wrong FP into the wrong module.

Copy link
Contributor

Choose a reason for hiding this comment

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

makes sense

{
// Send sys get ver check we have an answer
return readResponse(SYS_TABLE, SYS_TABLE, SYS_GET_VER, buffer, sizeof(buffer));
}

void TheThingsNetwork::autoBaud()
{
// Courtesy of @jpmeijers
Expand All @@ -396,19 +402,51 @@ void TheThingsNetwork::autoBaud()
while (attempts-- && length == 0)
{
delay(100);
// Send break + Autobaud
modemStream->write((byte)0x00);
modemStream->write(0x55);
modemStream->write(SEND_MSG);
sendCommand(SYS_TABLE, 0, true, false);
sendCommand(SYS_TABLE, SYS_GET, true, false);
sendCommand(SYS_TABLE, SYS_GET_VER, false, false);
modemStream->write(SEND_MSG);
length = modemStream->readBytesUntil('\n', buffer, sizeof(buffer));
// check we can talk
length = checkModuleAvailable();

// We succeeded talking to the module ?
baudDetermined = (length > 0) ;
}
delay(100);
clearReadBuffer();
modemStream->setTimeout(10000);
baudDetermined = true;
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 actually don't know why we added this flag to start with. Presumably to do a check before every sendCommand call and autoBaud if we didn't do so yet. If we do decide to implement this, we need to write the autoBaud and checkModuleAvailable functions not to use sendCommand.

But for now maybe let's just scrap the baudDetermined flag and rather introduce a moduleSleeping flag.

Copy link
Contributor

@hallard hallard May 16, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I think we need to know and distinct 2 cases

  • module available (physically connected and responding) if it's not we do nothing (no send)
  • module sleeping, we saw it at init (so available flag set), we put in in sleep (and we know that) but if it does not answer back, so trying to wake it up before sending any data, and check it answer

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point. Maybe not a flag then but an enum. Something like this perhaps? Please suggest naming.

enum state {
  UNKNOWN,
  AWAKE,
  SLEEPING
}

Copy link
Contributor

Choose a reason for hiding this comment

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

sounds good, may be UNKNOWN => NOT_FOUND or NONE or MISSING

}

bool TheThingsNetwork::isSleeping()
{
return !baudDetermined;
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 think we should rename this flag to something like moduleSleeping.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

}

void TheThingsNetwork::wake()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There isn't much difference between the wake() and autoBaud() functions. I would recommend we merge these two.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I do not wanted to change your original one, but I prefer also to merge both, much cleaner

{
// If sleeping
if (isSleeping())
{
// Send a 0 at lower speed to be sure always received
// as a character a 57600 baud rate
modemStream->flush();
#ifdef HARDWARE_UART
modemStream->begin(2400);
#endif
modemStream->write((uint8_t) 0x00);
modemStream->flush();
delay(20);
// set baudrate back to normal and send autobaud
#ifdef HARDWARE_UART
modemStream->begin(57600);
#endif
modemStream->write((uint8_t)0x55);
modemStream->flush();
modemStream->write(SEND_MSG);
if (checkModuleAvailable() > 0) {
baudDetermined = true;
}
}
}

void TheThingsNetwork::reset(bool adr)
Expand Down Expand Up @@ -969,11 +1007,9 @@ void TheThingsNetwork::sleep(uint32_t mseconds)
modemStream->write(buffer);
modemStream->write(SEND_MSG);
debugPrintLn(buffer);
}

void TheThingsNetwork::wake()
{
autoBaud();
// to be determined back on wake up
baudDetermined = false;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Again, moduleSleeping would make more sense here.

Copy link
Contributor

Choose a reason for hiding this comment

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

that's fine

}

void TheThingsNetwork::linkCheck(uint16_t seconds)
Expand Down
15 changes: 13 additions & 2 deletions src/TheThingsNetwork.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@

#define TTN_BUFFER_SIZE 300

// The Things Products devices
// Things Node only need this, we won't impact Things Uno user
#if defined(ARDUINO_THINGS_NODE)
typedef HardwareSerial SerialType;
#define HARDWARE_UART
#else
typedef Stream SerialType;
#endif

typedef uint8_t port_t;

enum ttn_response_t
Expand All @@ -42,7 +51,7 @@ enum ttn_fp_t
class TheThingsNetwork
{
private:
Stream *modemStream;
SerialType *modemStream;
Stream *debugStream;
ttn_fp_t fp;
uint8_t sf;
Expand All @@ -60,6 +69,7 @@ class TheThingsNetwork
void debugPrintIndex(uint8_t index, const char *value = NULL);
void debugPrintMessage(uint8_t type, uint8_t index, const char *value = NULL);

size_t checkModuleAvailable();
void autoBaud();
void configureEU868();
void configureUS915(uint8_t fsb);
Expand All @@ -78,7 +88,7 @@ class TheThingsNetwork
void sendGetValue(uint8_t table, uint8_t prefix, uint8_t index);

public:
TheThingsNetwork(Stream &modemStream, Stream &debugStream, ttn_fp_t fp, uint8_t sf = TTN_DEFAULT_SF, uint8_t fsb = TTN_DEFAULT_FSB);
TheThingsNetwork(SerialType &modemStream, Stream &debugStream, ttn_fp_t fp, uint8_t sf = TTN_DEFAULT_SF, uint8_t fsb = TTN_DEFAULT_FSB);
void reset(bool adr = true);
void showStatus();
size_t getHardwareEui(char *buffer, size_t size);
Expand All @@ -93,6 +103,7 @@ class TheThingsNetwork
ttn_response_t sendBytes(const uint8_t *payload, size_t length, port_t port = 1, bool confirm = false, uint8_t sf = 0);
ttn_response_t poll(port_t port = 1, bool confirm = false);
void sleep(uint32_t mseconds);
bool isSleeping();
void wake();
void saveState();
void linkCheck(uint16_t seconds);
Expand Down