IndigoDomotics / indigo-pushover

Indigo plugin to send push notifications via Pushover.
MIT License
17 stars 11 forks source link

Variables and more applications #1

Closed ghost closed 9 years ago

ghost commented 10 years ago

Hi,

I just started playing with Indigo and the Pushover plugin.

I noticed that I would like to do 2 things with the plugin that are not (yet) possible, unless I missed something:

  1. I would like to include a variable text in the message field. Something like 'this is a message'. Not sure if/how this is possible but it would make the plugin much more flexible.
  2. I have also tested openHAB. Here it is also possible to send to more than only just one application. Would it be possible to implement something like this?

When you can point me to where to look I might be able to help/implement this...

Cheers,

Marcel

ghost commented 10 years ago

I created my own version of this plugin supporting an application token per trigger (when empty default is used), and I added optional values sound, device, url and url title.

I also added support for replacing variables in indigo format (%%v:VARID%%) in title and message. Use of more variables and nested within variables is also supported.

The only thing I did not do is evaluate the variable existence...

When you want I can send you this stuff so you can evaluate it.

I added a screenshot of the settings window as it looks now.

screen shot 2014-04-02 at 16 53 58

dwradcliffe commented 10 years ago

Using variables would be great!

yamanote1138 commented 10 years ago

@IT2BE that sounds great! please open a pull request with your changes and I'll happily incorporate your features into a new release.

ghost commented 10 years ago

@discgolfer1138 the pull request stuff was new to me but I think I managed to do it :)

yamanote1138 commented 9 years ago

sorry this has taken me forever to review! I totally agree with your implementation of variables and the ability to customize sound is a nice touch.

The only thing I'm not really wild about is the optional application token, it seems unnecessary. Can you tell me a use case where you'd need more than one application token?

ghost commented 9 years ago

Hi Chad, there are two reasons for the implementation of an optional token:

  1. It is there and I wanted to implement everything that makes sense.
  2. Each Pushover Application has its own token and settings. I use this to immediately see, by the used icon, what type of urgency the message has. But it also gives you statistics per type of message which could be handy. Example: how many times was my alarm triggered this month in my absence or whatever you can think of.
yamanote1138 commented 9 years ago

The more I think about it, if you want to be able to send pushover notifications to different applications, you can simply create additional plugin instances for each application. This would have the added benefit of not having to change all your actions if the api key changes.

I will implement your other changes into the next version, but I think I'm going to pass on the multiple application key idea. I'll merge your changes manually and close the pull request. I will be sure to give you full credit in the version notes.

yamanote1138 commented 9 years ago

also, in regards to denoting urgency, I am implementing the ability to set message priority since the api now supports it.

ghost commented 9 years ago

Urgency was one of those things that I do/did not see a use for but I implemented it for a user with a sick kid.

How would you implement more instances of a plugin for this? As a device because I find that 'ugly'. Like the SecuritySpy Server. I can see why it was done that way but it feels like abusing a device. But that is my opinion :)

yamanote1138 commented 9 years ago

Totally get what you're saying now about the need for the ability to optionally override application key. I've been away from Indigo plugin development and, for some odd reason, I had mistakenly deluded myself that the ui allowed you to have multiple instances of a plugin. I was totally thinking of devices.

I completely agree that try to implement this plugin as a device seems an abuse of the device concept. As an aside, I think the ability to have multiple instances of a plugin might be kinda handy.

While I do now have a better understanding of your use case, I wish there was a better way to implement it than just a text field that overrides the key. That would mean users would have to copy/paste their keys into every action instance and that could get very messy very fast in a more complex setup.

If you're cool with it, I'd like to get a new version out and sort out the multiple app key thing later. I created a new branch in my repo and manually integrated your changes. I'm sure you'll notice I renamed a bunch of the vars and reordered the logic a bit... I also moved the variable injection into a wrapper function because I have another issue I need to resolve there (namely character encoding).

Anyway.. that was a lot of blathering on my part. Hope you're cool with all my nonsense. I really appreciate your contributions and look forward to working together to figure out the best way to implement multiple app ids.

yamanote1138 commented 9 years ago

also, I'm likely going to move this repo to Indigo's new org once I've stabilized the build a bit.

ghost commented 9 years ago

Yeah, I would move it to Indigo too if I were you. Makes sense :)

I know people are using the application key so be careful with not implementing it.

I get what you are saying about copy/paste stuff but I don't think that is too much of an issue. When you think it is why not add a field to the plugin setup with a 'list' of all application keys. Make the keys comma or new line separated and use it to create a menu list for when a user creates an action of it. Should have thought of that to begin with I guess ;)

When you want I can do that tonight or tomorrow night.

BTW encoding was implemented...

yamanote1138 commented 9 years ago

that's a great idea! I'll roll that into this version :)

ghost commented 9 years ago

Added benefit is that it will not break anything when you leave the field name intact (I hope)...

yamanote1138 commented 9 years ago

correct me if I'm wrong, but since I've never implemented that feature... only users running your fork of the plugin would be affected, yes?

ghost commented 9 years ago

Correct! Not sure how many that are though...

In any case, I never officially released it so when you add a disclaimer there should be no issue...

yamanote1138 commented 9 years ago

I figure this is free software and a certain degree of 'use at your own risk' is reasonable :)

I have a stable release almost ready that implements all of your changes (except the app key override). Just need to tidy up the documentation and the helper text in the action config ui.

yamanote1138 commented 9 years ago

I've given you collaborator access to this repo, the new branch I'm working on is just called v1.1.2 if you want to try implementing that optional application key override...

Got a big day tomorrow, not sure when I'll be able to get back in there.

ghost commented 9 years ago

Hi Chad, I added the changes to my own repo. Like you I don't have too much time right now (preparing for a week off in London, attending Kate Bush concert :) ).

Anyway, it seems to work as expected and previously set keys seem to 'stick' except for default empty keys...

yamanote1138 commented 9 years ago

Very cool! I think I'm going to keep my version simple and retain just the single application key. As such, I'm going to close the pull request as my codebase has diverged rather significantly at this point. Thank you so much for your contributions, I will be sure to give you due credit!

ghost commented 9 years ago

Hi Chad, I understand but that means that I will keep my version of the plugin alive because I really use the api keys.

yamanote1138 commented 9 years ago

Of course! Whatever works best for you :)

On Thu, Aug 28, 2014 at 1:30 AM, Marcel Trapman notifications@github.com wrote:

Hi Chad, I understand but that means that I will keep my version of the plugin alive because I really use the api keys.

Reply to this email directly or view it on GitHub: https://github.com/discgolfer1138/indigo-pushover/issues/1#issuecomment-53682547

ghost commented 9 years ago

Hi Chad,

One more attempt :) I think I made it so that it is easier to use. The user can now add a name/key pair so that using/selecting the correct application key makes much more sense. I attached 2 screenshot but it is still up to you obviously :) screen shot 2014-08-28 at 23 28 49 screen shot 2014-08-28 at 23 29 06