Skip to content

Bugfix and add Low Power Mode #12

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

Merged
merged 12 commits into from
Feb 13, 2018

Conversation

hallard
Copy link
Contributor

@hallard hallard commented Feb 12, 2018

- Added Hardware Reset of lora module at startup
- Fixed push button duration bug
- Set IRQ var to be volatile
@CLAassistant
Copy link

CLAassistant commented Feb 12, 2018

CLA assistant check
All committers have signed the CLA.

@hallard
Copy link
Contributor Author

hallard commented Feb 12, 2018

This new pull request #12 should fix your issue, just let me know

@johanstokking johanstokking self-requested a review February 12, 2018 10:04
@johanstokking johanstokking self-assigned this Feb 12, 2018
Copy link
Member

@johanstokking johanstokking left a comment

Choose a reason for hiding this comment

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

Thanks @hallard !! Looks good. Few changes, mostly styling. I'd like to keep this repository clean and consistent.

@@ -1,5 +1,5 @@
name=TheThingsNode
version=2.0.2
version=2.0.3
Copy link
Member

@johanstokking johanstokking Feb 12, 2018

Choose a reason for hiding this comment

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

We're bumping versions in the master branch after merging – sometimes we combine PRs in a version

@@ -72,9 +73,12 @@ void TTN_BUTTON_FN()
uint8_t trigger = getPinChangeInterruptTrigger(digitalPinToPCINT(TTN_BUTTON));
if (trigger == FALLING)
{
//Serial.println(F("IPRESS"));
Copy link
Member

Choose a reason for hiding this comment

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

Remove if not used

// Don't go to sleep mode while button is still pressed
// because if so, timer of ms will be stopped and duration
// of pressed button will not work
if ( this->buttonPressed ) {
Copy link
Member

Choose a reason for hiding this comment

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

Formatting (spaces)

@@ -737,6 +749,15 @@ TheThingsNode::TheThingsNode()
pinMode(TTN_BLUE_LED, OUTPUT);
setColor(TTN_BLACK);

// set pin mode every time to avoid constraining the user about when the pin is initialized
#ifdef TTN_LORA_RESET
Copy link
Member

Choose a reason for hiding this comment

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

Formatting (indentation)

@@ -81,6 +81,9 @@ void sleep() {
}
```

Pay attention, this internal sleep method of the library does not put the Lora module (RN2483) into sleep mode and thus your node may consume 3mA even in sleep mode. You need to manually set the lora module to sleep and wake.
Copy link
Member

Choose a reason for hiding this comment

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

Lora -> LoRa
lora -> LoRa

Copy link
Member

Choose a reason for hiding this comment

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

RN2483 can also be RN2903

@@ -140,4 +143,11 @@ void sendData(uint8_t port)
payload[5] = bytes[0];

ttn.sendBytes(payload, sizeof(payload), port);

// Set RN2483 to sleep mode
ttn.sleep( 60000 );
Copy link
Member

Choose a reason for hiding this comment

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

Formatting (spaces)

ttn.sendBytes(lpp.getBuffer(), lpp.getSize(), port);

// Set RN2483 to sleep mode
ttn.sleep( (uint32_t) (CONFIG_INTERVAL*1000) );
Copy link
Member

Choose a reason for hiding this comment

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

Formatting (spaces)

@@ -0,0 +1,172 @@
// **********************************************************************************
Copy link
Member

Choose a reason for hiding this comment

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

Although useful, we're not using these headers in examples.

Please add the concerning comments in code, if necessary.

@@ -122,7 +122,7 @@ void sendData(uint8_t port, uint32_t value)
ttn.sendBytes(lpp.getBuffer(), lpp.getSize(), port);

// Set RN2483 to sleep mode
ttn.sleep( (uint32_t) (CONFIG_INTERVAL*1000) );
ttn.sleep( CONFIG_INTERVAL*1000 );
Copy link
Member

Choose a reason for hiding this comment

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

I meant to say no spaces in the parentheses 😊

@johanstokking
Copy link
Member

Can you also fix the CI?

@hallard
Copy link
Contributor Author

hallard commented Feb 13, 2018

I've done all modifications, travis should be fine on next check

@johanstokking
Copy link
Member

johanstokking commented Feb 13, 2018

@hallard you're using the wake() and sleep() functions which are not in the referenced release for testing of TheThingsNetwork.

Please change .travis.yml#L14 to version v2.5.6

@hallard
Copy link
Contributor Author

hallard commented Feb 13, 2018

It's done, sorry wrong comments
Requires TheThingsNode library 2.5.6
should be
Requires TheThingsNetwork library 2.5.6

@johanstokking johanstokking changed the base branch from master to feature/low-power-mode February 13, 2018 16:17
@johanstokking johanstokking merged commit 1fa12f4 into TheThingsNetwork:feature/low-power-mode Feb 13, 2018
@johanstokking
Copy link
Member

Thanks, this is now part of https://github.com/TheThingsNetwork/arduino-node-lib/releases/tag/v2.0.5

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