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.

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

SMS Sender / Receiver #522

Merged
merged 7 commits into from Jun 17, 2019
Merged

SMS Sender / Receiver #522

merged 7 commits into from Jun 17, 2019

Conversation

glmnet
Copy link
Member

@glmnet glmnet commented Apr 27, 2019

Description:

This is the Sim800L Component.

so far it can receive and send sms.
It does not work with software serial due to the buffer is only 64 bytes.

test yaml:

# this uart config will not work in ESP8266 due to software serial buffer is too small
uart:
  tx_pin: D0
  rx_pin: D1
  baud_rate: 9600

sim800l:
  id: sms
  on_sms_received:
    lambda: |-
      ESP_LOGD("main", "Received sms from %s: %s", sender.c_str(), message.c_str());

binary_sensor:
  platform: gpio
  name: "Send sms"
  pin: D8
  on_press:
    lambda: |-
      id(sms).send_sms("+your phone #", "Hello there from ESPHome");

@OttoWinter about Software Serial, do you think I could add an optional software_buffer_size so the 64 can be overridden in yaml? I'm pretty sure the ESP8266 can allocate more memory for this and still run reliable.

Related issue (if applicable):
esphome/feature-requests#166

Pull request in esphome-docs with documentation (if applicable): esphome/esphome-docs#

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

Copy link
Member

@OttoWinter OttoWinter left a comment

Choose a reason for hiding this comment

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

Looks quite good to me now!

Want to add an SMS send action too? It would be pretty simple

template<typename... Ts>
class Sim800LSendAction : public Action<Ts...> {
  public:
    TEMPLATABLE_VALUE(std::string, recipient)
    TEMPLATABLE_VALUE(std::string, message)

    void play(Ts... x) {
      auto recipient = this->recipient_.value(x...);
      auto message = this->message_.value(x...);
      this->parent_->send_sms(recipient, message);
   }
};

And python side would also be simple (see usages of automation.register_action).

}),
}).extend(cv.polling_component_schema('5s'))
.extend(uart.UART_DEVICE_SCHEMA)
.extend(cv.COMPONENT_SCHEMA))
Copy link
Member

Choose a reason for hiding this comment

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

This extend is not necessary (polling_component_schema already extends COMPONENT_SCHEMA)

@glmnet
Copy link
Member Author

glmnet commented May 7, 2019

Thanks I'll do.
Do you want me to squash all commits and force push again the branch? or that is not necessary? (I'm nb on git collaboration yet)

@glmnet
Copy link
Member Author

glmnet commented May 8, 2019

binary_sensor:
  platform: gpio
  name: "Send sms"
  pin: D8
  on_press:
    - sim800l.send_sms:
        id: sms
        recipient: "+5493624207774"
        message: Hello from ESPHome yaml :P

def sim800l_send_sms_to_code(config, action_id, template_arg, args):
paren = yield cg.get_variable(config[CONF_ID])
var = cg.new_Pvariable(action_id, template_arg, paren)
template_ = yield cg.templatable(config[CONF_RECIPIENT], args, str)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
template_ = yield cg.templatable(config[CONF_RECIPIENT], args, str)
template_ = yield cg.templatable(config[CONF_RECIPIENT], args, cg.std_string)

I don't think str will work (at least it should throw a ValueError because there are several string types and we don't know which one should be used: std::string, const char *, String etc)

Copy link
Member Author

Choose a reason for hiding this comment

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

It did work, the sample yaml in the previous message. I'll change it anyway.

Copy link
Member

Choose a reason for hiding this comment

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

I see. Yes the example does work - however it will fail for templated !lambda values.

@OttoWinter
Copy link
Member

Do you want me to squash all commits and force push again the branch? or that is not necessary?

You can add as many commits as you want. Github automatically squashes it when I click merge ("Squash and merge")

@glmnet
Copy link
Member Author

glmnet commented May 8, 2019

By the way, my change of uart serial buffer size slipped in some commits ago.
I modified from 64 to 512 (just to be safe) I can revert that so you don't have to cherry pick.
In the other hand to make this component usable with software serial it wont work with buffer 64 as message are longer and software serial receive code fills buffer before this component has a chance to empty it.
One possible solution would be to make the buffer size adjustable.

# Example configuration entry
uart:
  tx_pin: D0
  rx_pin: D1
  baud_rate: 9600
  software_buffer_size: 512  # optional, default 64

@OttoWinter
Copy link
Member

By the way, my change of uart serial buffer size slipped in some commits ago.

That's fine I think - I haven't really seen instances of people hitting the memory limit yet. We can always change it back if needed.

For this PR: I'd like to cut the 1.13 release soon - do you think this PR is ready in the current state? SMS support would be an awesome feature to have.

I can also manually cherry-pick it into the beta branch later too - and since beta period will probably be a bit longer this time you don't have to hurry that much.

@glmnet
Copy link
Member Author

glmnet commented May 28, 2019

This code works, It still needs some debugging as as soon a SMS is received it cannot parse it, it retries due to the constant polling and then it parses properly, so I'm still working on that. I plan to finish debugging this week and also get the docs done.

Copy link
Member

@OttoWinter OttoWinter left a comment

Choose a reason for hiding this comment

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

Hi,
looks like your git branch has a wrong merge on it - see the files tab, files that are not part of this PR are showing up.
Please see the instructions in the contributing guide for the correct merging/rebasing.



SIM800L_SENS_SMS_SCHEMA = cv.Schema({
cv.Required(CONF_ID): cv.use_id(Sim800LComponent),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
cv.Required(CONF_ID): cv.use_id(Sim800LComponent),
cv.GenerateID(): cv.use_id(Sim800LComponent),

This way the user doesn't need to type it and the first one in the config is automatically chosen.


SIM800L_SENS_SMS_SCHEMA = cv.Schema({
cv.Required(CONF_ID): cv.use_id(Sim800LComponent),
cv.Required(CONF_RECIPIENT): cv.templatable(cv.string),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
cv.Required(CONF_RECIPIENT): cv.templatable(cv.string),
cv.Required(CONF_RECIPIENT): cv.templatable(cv.string_strict),

@glmnet glmnet force-pushed the gsm branch 2 times, most recently from 3ebf118 to f906b50 Compare June 8, 2019 00:52
@glmnet
Copy link
Member Author

glmnet commented Jun 8, 2019

Ok I guess I applied all request here and in the docs. I had to force push them.
The sms code is working reliably on my esp8266 I might improve that later though.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants