WIP: pluggable upload destinations

[tedder]

Since uploads.im was going away I wanted to store my images on S3. I'm sure that's not what most people want to do, so I abstracted it a little. This strategy would be a good way to go for sending messages (eg twilio, gvoice).

Marking as a WIP so you all can see it, and because it's a WIP. I'm still testing, and I need to add options to the config panel.

[taxilian]

@shadycuz I don't have time to review this right now, any chance you do?

[shadycuz]

Looks like a really nice upload class. Lots of flexibility for the future. @tedder Let me test this, and fleshout what the UI should look like in the settings tab. I'll push any changes to your branch.

Thanks, Levi

[tedder]

Thanks. I'm still iterating and testing, I'll push my fixes now.

[shadycuz]

@tedder oh yes please do what you do ;)

I am torn about the new function for sending a text with image. To me the body of the text stays the same and the image is optional. Which is why I used the one function with optional url.

Your new function is cleaner but maybe unnecessary? Maybe it would help to move the message formatting to it's own function and pass the body into the function?

For example we allow messages to go to more than one number, so maybe we have a function that produces a dict where the keys are numbers and the values are the formatted txt message.

{
   "+1-555-555-5555" : "This would be the text sent"
}

Doesn't even look like multiple numbers is working, I probably broke it 👎

https://github.com/taxilian/OctoPrint-Twilio/blob/5e1dd4fe2a7f20612512f8631b3c6d14ffdd6488/octoprint_smsnotifier/__init__.py#L100-L109

I'm just thinking out loud.

[tedder]

What I was trying to do with the second send() function was to centralize the "try with image, then try without image" stuff. It should work for all cases now, right?

I haven't used the multiple phone stuff. Once I'm sure it's actually working I'll look at that. Message formatting elsewhere is a decent idea.

[shadycuz]

I thought it worked for all cases before?

[tedder]

@shadycuz I fixed the 'multiple phone numbers' thing.

I've been using this branch constantly since putting in the PR, works well with S3.

[tedder]

I'm going to be setting up a few Pis in the next week and want this feature- can you merge or should I just fork the plugin?

[shadycuz]

We want your continued development in here. Let me get some coffee and look it over again and then I'll merge.

[shadycuz]

@tedder I'll get this merged this weekend.

[taxilian]

@shadycuz can't help notice that you indicated you were going to merge this but didn't; is it ready to be merged?

[shadycuz]

@taxilian I will try it again soon.