app_sms.c: Fix sending and receiving SMS messages in protocol 2 #1188
Conversation
|
|
github-actions
bot
added
pr-submit-testing-in-progress
pr-submit-tests-passed
has-pr-checklist
|
Workflow PRCheck completed successfully |
|
There's a comment saying that I didn't sign the CLA. I signed the CLA a few minutes ago, I'm not sure if this is a refresh error. |
|
Is there something else that I ought to do? If so, please let me know. Thanks! |
|
The automated message you are referring to only updates when changes are pushed to the PR. There is nothing for you to do until this is reviewed. |
apps/app_sms.c
Outdated
| <para>It is also important to adjust the gain dB of the ATA. In one of my Telefónica DOMO Mensajes phones, | ||
| I had to set the dB level to +3dB, and in another phone of the same model, I had to set the gain to +6dB | ||
| to get it to work.</para></note> |
There was a problem hiding this comment.
If possible, rewrite this bit to avoid first-person phrasing.
There was a problem hiding this comment.
Sorry, I was not aware that I had to do that. Would the following sentence be acceptable?
It is also important to adjust the gain dB of the ATA. Some Telefónica DOMO Mensajes phones may require the gain to be set to +3dB,
and others even up to +6dB, in order to work.
There was a problem hiding this comment.
Ok, I made a commit to fix this. This is my first time at contributing, so I'm a little lost. If there's anything else I should do, please let me know. Thanks!
github-actions
bot
added
pr-submit-testing-in-progress
pr-submit-tests-passed
and removed
pr-submit-tests-passed
pr-submit-testing-in-progress
labels
|
Workflow PRCheck completed successfully |
|
I've done the changes as needed. I was wondering if there was anything else for me to do... if so, please let me know. Thanks! |
|
There is nothing for you to do until this is reviewed. |
There was a problem hiding this comment.
A few housekeeping things...
- Move the "UserNote" in both the commit message and the PR description to the end. Otherwise the whole commit message will become the user note. Strictly speaking, you don't actually need the UserNote for this commit so you could just remove it altogether if you want.
- Squash the two commits into one using
git rebase -i HEAD~2and changing the command for theFix the first use...commit tofixup. Then do agit push --force.
|
Oh, you should also cherry-pick this PR to the 20, 21 and 22 branches. |
github-actions
bot
added
pr-submit-testing-in-progress
pr-submit-tests-failed
and removed
pr-submit-tests-passed
pr-submit-testing-in-progress
labels
|
Workflow PRCheck failed |
github-actions
bot
added
pr-submit-testing-in-progress
and removed
pr-submit-tests-failed
labels
|
Hi, are there any updates on this? Is there anything else I ought to do? |
|
There are no updates or requests that I see. If something is needed then something will comment. As it is this is pending review and approval by someone else. |
|
This PR has been marked stale because it has been in "Changes Requested" or "submitter-action-required" state for 28 days or more. Please make the requested changes within 14 days or the PR will be closed. |
|
I don't understand, why has this issue been marked as stale? I thought I had no more changes to do from my part? Then how come it's in "Changes requested"? |
|
George was still marked as having changes requested, I've removed the stale flag and re-requested review from him. |
github-actions
bot
added
cherry-pick-testing-in-progress
github-actions
bot
added
cherry-pick-testing-in-progress
|
Workflow PRCPCheck failed |
|
Successfully merged to branch master and cherry-picked to ["20","21","22"] |
This fixes bugs in SMS messaging to SMS-capable analog phones that prevented app_sms.c from talking to phones using SMS protocol 2.
One of the bugs caused messages to have random characters and junk appended at the end up to the character limit. Another bug prevented Asterisk from sending messages from Asterisk to the phone at all. A final bug caused the transmission from Asterisk to the phone to take a long time because app_sms.c did not hang up after correctly sending the message, causing the phone to have to time out and hang up in order to complete the message transmission.
This was tested with a Linksys PAP2T and with a GrandStream HT814, sending and receiving messages with Telefónica DOMO Mensajes phones from Telefónica Spain. I had to play with both the network jitter buffer and the dB gain to get it to work. One of my phones required the gain to be set to +3dB for it to work, while another required it to be set to +6dB.
Only MORX and MTTX were tested, I did not test sending and receiving messages to a TelCo SMSC.