michaelkourlas / voipms-sms-client

Popular Android messaging app for VoIP.ms, a Canadian VoIP provider
https://play.google.com/store/apps/details?id=net.kourlas.voipms_sms
Apache License 2.0
216 stars 51 forks source link

Add a few MMS fixes. #246

Closed KenMacD closed 1 year ago

KenMacD commented 1 year ago

In testing the mms branch I ran in to a few issues that these commits fix:

(Re: #190)

KenMacD commented 1 year ago

Hi @michaelkourlas. I've been using a local build with the above for over a month now without issue. Any feedback on this PR. or MMS support in general? (I'm trying to decide if I want to set up push notifications)

michaelkourlas commented 1 year ago

For copyright and maintenance reasons I prefer to write my own code, so it's unlikely I'll end up merging this PR.

I don't have any immediate term plans to finish the MMS work, so if you'd like to add push messaging support for your own use in the meantime, please do!

KenMacD commented 1 year ago

Okay, thanks @michaelkourlas. Personally with these changes I'd consider the MMS work 'finished'. I'm not sure what else you had planned, but for me it works a treat.

As for the above changes, it's just a couple of simple fixes and I couldn't care less about the copyright or authorship, so feel free to copy/paste them to your own commit. I hereby multi-license this code under WTFPL, CC0, Unlicensed, and MIT-0.

yulman19 commented 1 year ago

@KenMacD If @michaelkourlas doesn't want to do the pull, could you release a build?

JohnMertz commented 1 year ago

@KenMacD you don't need to declare a new license. It is Apache-2.0, which states, in part:

5. Submission of Contributions. Unless You explicitly state otherwise, any Contribution intentionally submitted for inclusion in the Work by You to the Licensor shall be under the terms and conditions of this License, without any additional terms or conditions.

Which is fine because this means you contribution is under the same copyright terms, notably:

2. Grant of Copyright License. Subject to the terms and conditions of this License, each Contributor hereby grants to You a perpetual, worldwide, non-exclusive, no-charge, royalty-free, irrevocable copyright license to reproduce, prepare Derivative Works of, publicly display, publicly perform, sublicense, and distribute the Work and such Derivative Works in Source or Object form.

So there is no real copyright conflict. I'm not sure that I understand the objection (nor would I want to speculate) given that other contributions have been merged in the past. Presumably the argument about maintainability could be valid if you are introducing a feature which could create a larger support burden or which is too big or complex to be brought under one individuals maintainership. The former would be an argument to adopt plugin functionality with separate issue tracking, while the later would be for co-maintainership, or even a community fork if absolutely necessary.

KenMacD commented 1 year ago

@KenMacD If @michaelkourlas doesn't want to do the pull, could you release a build?

@yulman19 To avoid the numerous issues that would cause, I was hoping to avoid creating one. That's partially why I asked for feedback here. If required I could generate one though.

@KenMacD you don't need to declare a new license. It is Apache-2.0...

@JohnMertz well, I'm not a lawyer and playing devils advocate, I'm not sure how much weight I'd put in me having implicitly entered in to a contract based on a license I haven't even read let alone signed anything. If I make my repo license state that contributors agree to pay me $1M for every PR opened I doubt I'll make any money. Maybe the conflict is around the relicensing mess is the problem (eg apparently Apache-2.0 isn't compatible with GPL-2.0)?

But as I said, I couldn't care less about authorship of these tiny fixes. Apache-2.0, copy/paste, whatever. I just needed mms support to pass the 'kid test', like others in #190. Heck feel free to copy/paste and then put in the changelog that the mms code was delayed because of me if you want.

michaelkourlas commented 1 year ago

Yes, it's not clear to me whether section 5 of the license is actually enforceable in practice. The right way to do things would be to obtain a signed contributor license agreement or copyright assignment, but I don't really want to go through that trouble.

With respect to maintenance, I tend to find it easier to maintain code that I wrote.

Beyond those two reasons, there's also the simple fact that as this is a hobby project, I prefer to write all of the code myself and have complete control and ownership over it. That's just part of the appeal for me.

It's true that I have accepted PRs in the past. However, they were very small, and even so I probably wouldn't accept them today.

KenMacD commented 1 year ago

It's true that I have accepted PRs in the past. However, they were very small, and even so I probably wouldn't accept them today.

Hopefully not getting too far off topic here, but might I kindly suggest putting that in a CONTRIBUTING.md to warn anyone who might be considering spending time on substantial changes?

michaelkourlas commented 1 year ago

Yep, that's a good idea. I'll put something together.

TurboLed commented 1 year ago

Hi I'm waiting for a build of the MMS support, as I cannot succeed to build locally, I'm running into issues with securestoragelibrary as well as with Firebase. If one of you can provide a release for the MMS branch or with instructions on how to build locally with Android Studio that would be much appreciated! Lacking the app, I ran into crazy MMS fees by forwarding all the incoming MMS to my cell phone 👎

xenotropic commented 1 year ago

This seems tragic. Aren't we talking about four lines of code here? Is there any publicly-available apk of this fork? @KenMacD you previously said "If required I could generate [a build]", just wondering if that is still possibly on your to-do list.

KenMacD commented 1 year ago

@xenotropic Last month I reluctantly posted a debug build on my fork. It doesn't use the Google APIs for notifications (I don't have the key for that), so you'll have to weigh mms against quicker notifications.(I'd like to add UnifiedPush to it eventually).

slevind commented 1 year ago

Hi @KenMacD , I have downloaded and installed the debug build. Thank you. I have noticed that I still get an empty line occasionally when there is only a picture attached to the message. Looking forward to the push notifications.

Is there also the ability to add an attach file to the message so a user can send MMS picture messages?

Cheers. Keep up the awesome work.

xenotropic commented 1 year ago

@KenMacD Your debug build is working pretty good for me, thank you. Do you know if the limiting factor for multi-recipient MMS ("group texts") is the VoiP.ms API -- i.e., that the information just isn't available? Or is the information there to compile group messages into threads, but no one has had the time to incorporate it into the app? Separately, you might also consider turning on issues on your fork.