Use Qksms existing send functionality #10
Conversation
…th is taken care of
timbze
changed the title
|
Is it possible to make saving messages configurable? |
Yes, I have pushed another commit to do that now. It will not save by default, but you can (optionally) send |
|
It would of course also be possible through a app setting, but would be a bit more work than how I did it now |
| "to" -> phone = reader.nextString() | ||
| "message" -> message = reader.nextString() | ||
| "saveMessage" -> saveMessage = reader.nextBoolean() |
| @@ -299,8 +299,10 @@ class MessageRepositoryImpl @Inject constructor( | |||
| addresses: List<String>, | |||
| body: String, | |||
| attachments: List<Attachment>, | |||
| delay: Int | |||
| delay: Int, | |||
| saveSentMessage: Boolean | |||
There was a problem hiding this comment.
Is there any way to avoid so many API changes? This might cause merge conflicts in the future as we merge upstream QKSMS. Maybe we can pass information in some other way? Like maybe pass a special value in addresses or attachments to indicate if message should or should not be saved? It's a bit hacky, but would save time later.
There was a problem hiding this comment.
Possible, probably. Feasible, not in my opinion. This place in code is only used a few times. I think if I make changes in multiple methods with the addresses or attachments variable there would be just as much chance for conflicts as it is now, and would be more prone to have other negative side effects.
There was a problem hiding this comment.
OK, how about moving the new parameter to the top of the methods. That way chance of conflict is lower. And also cleaner diff.
There was a problem hiding this comment.
I did one now, but the one for SendMessage.kt Params has more usages where I have to go change them all, so I'm not sure changing that one is a good idea.
There was a problem hiding this comment.
I see. OK, I think we are good to merge. Thank you for the pull request and fixing my comments.
| @@ -40,7 +40,8 @@ class SendMessage @Inject constructor( | |||
| val addresses: List<String>, | |||
| val body: String, | |||
| val attachments: List<Attachment> = listOf(), | |||
| val delay: Int = 0 | |||
| val delay: Int = 0, | |||
| val saveSentMessage: Boolean = true | |||
There was a problem hiding this comment.
Please move this to the top as well.
|
Thnak you for the PR and merge guys! |
Fixes #7
Fixes #9
This changes Traccar sms sending so that it uses Qksms functionality. That way sent message is saved and message is split if needed
This would also make #5 and #8 irrelevant. I believe this is a better approach
@tananaev