-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes, Style stuff and Feature enhancements: #23
Conversation
* reworked modem initialization that order of commands is correct after PIN * add support to check and provide a PIN * added an optional logger via options; currently supports "debug" logging * added debug logging for Writes and Received commands * added a custom initialization command (some modes need that) which is then send after PIN check * added options.incomingSMSIndication flag to officially send enable command for push info on new messages; default is true for backward compatibility * changed double-quotes into single quotes in strings * send AT+CMEE=1 (enhanced error infos) and CREG=2 (enhanced details on CREG command) * make some checks more generic * add error details to the error messages * some code and logic error from last refactoring (was not really working on some places?) are fixed * add signalStrength property with calculated db value on Signal-Strength method
Requesting review @karianpour, @zabsalahid |
One note: I'm not able to get "Incoming SMS or Call notifies", but It may be my device ... so please check that carefully on your side too |
* executeCommand always need to return an object, else all the "item.logic" calls will end in an exception * removed unneeded comma in some objects
* rename introduced "checkModem" to "resetModem" because ATZ will also reset to factory defaults! * add a new checkModem to only send AT to check communication with device
data: {pinNeeded: !newpart.includes('READY')} | ||
} | ||
} | ||
if (newpart.includes('OK') && resultData) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Apollon77 not all modem returns OK
. Some only return +CPIN: READY
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
really? I always get (and found in docs) +CPIN: READY and after that an emoty line and then OK.
If it is really the case as you say then the whole "detect when the command is done" to really send the result is unreliable and we need to change it completely ... But what's the alternative to detect that one command got answered finaly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One idea could be to usilize the current "ECHO" behaviour because (at least for me) all send commands are echoed. So we would need to change the logic so that we check for the appearence of the "ECHO" from the next command in the queue and if this is received without the last queue entry was send we e.g. inject a "virtual" "OK" line (just to finish old command successfully). What do do you think about this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Apollon77 Yep, I was testing it out, but the initialization was taking too long, and there it is, it didn't go through here:
if (newpart.includes('OK') && resultData) {
return {
resultData,
returnResult: true
}
}
Yep, the docs says after +CPIN: READY, there would be an 'OK' after. Mine wouldn't. I'm using
these, they're all from China I think:
and
@Apollon77 How would we also know it it only took a long time for the command to finish, or was there an 'OK' at all.
"But what's the alternative to detect that one command got answered finaly?", I have no idea too, maybe a list of commands and possible results from the modem? An input from the package users, and creating a dictionary out of it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Apollon77 for the meantime, I'll remove newpart.includes('OK')
and format the document. And will push and publish after some more checking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I will not do an "OK" but an "Onw magic String" like "COMMAND_END" that will not really some as data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oooh even other idea ... I add a new flag to the item if it is "active" or not and only then the check logic gets executed ... so we wait for the echo nd "activate" the item then for logic processing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you saying it like this?
if (newpart.startsWith('+CPIN: ')) {
resultData = {
status: 'success',
request: 'checkPINRequired',
data: { pinNeeded: !newpart.includes('READY') }
}
modem.port.write(`${item.command}OK\r`)
}
if (newpart.includes(`${item.command}OK`) && resultData) {
return {
resultData,
returnResult: true
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No ... let me try something ... :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check #25 ... maybe that way? :-) Feels sexy to me
reworked modem initialization that order of commands is correct after PIN
add support to check and provide a PIN
added an optional logger via options; currently supports "debug" logging
added debug logging for Writes and Received commands
added a custom initialization command (some modes need that) which is then send after PIN check
added options.incomingSMSIndication flag to officially send enable command for push info on new messages; default is true for backward compatibility
changed double-quotes into single quotes in strings
send AT+CMEE=1 (enhanced error infos) and CREG=2 (enhanced details on CREG command)
make some checks more generic
add error details to the error messages
some code and logic error from last refactoring (was not really working on some places?) are fixed
add signalStrength property with calculated db value on Signal-Strength method
add proper error handling when accessing empty SMS slots
executeCommand always need to return an object, else all the "item.logic" calls will end in an exception
removed unneeded comma in some objects
fix checkSimMemory
rename introduced "checkModem" to "resetModem" because ATZ will also reset to factory defaults!
add a new checkModem to only send AT to check communication with device
parse SimInbox response synchronous to prevent pot. async problems (in fact now method can do both, the event is emitted async)
fixes Enhancements for Huawei K3765 HSPA stick #18, Return signal strength also as dB value #19
partially getNetworkSignal() does not return any data #20