modem.dataReceived function is too complicated

[karianpour]

I suggest to move some logic parts of the code in modem.dataReceived to the caller function. For example modem.checkSimMemory function is calling modem.executeCommand function, there we add an item in the modem.queue, and then in the modem.dataReceived we get data and we check for modem.queue[0].command == 'AT+CPMS="SM"' if it matches, if yes then comes the logic to proceed further. I suggest to move the logic block in dataReceived function which is the follow :

            if (newpart.trim().substr(0, 6) === '+CPMS:') {
              modem.parseSimCardResponse(newpart, result => {
                resultData = result
              })
            }
            if ((newpart == ">" || newpart == 'OK') && resultData) {
              returnResult = true
            }

as and arrow function inside modem.checkSimMemory function. I can do it if you agree.

[zabsalahid]

@karianpour I think so, lots of my codes haven't been arranged well yet. Man, you're very helpful on this project, thank you so much.

[karianpour]

@zabsalahid I prepared a sample refactoring, can you please check the draft pull request.

[Apollon77]

I would completely second that. I think especially all command that only give one line as standard response could call "execute" with a parameter so that the first response line after the command is given directly to the caller as result and so these methods could parse this by themself.

Also all normal commands are "AT+Command" and the Response starts with "Command" so this also can be auto detected. This should reduce lot of complexity.

Then only a solution wuld be needed for "unknownnumer of lines" as response ... does this exist?

[Apollon77]

I have no idea how far you are with your refactoring plans and if someone is actively working on, I could also offer my help - but we shoukd sync that not multiple persons work on the same topic

[zabsalahid]

Hey @Apollon77 !

I would appreciate any help. At the meantime, only @karianpour is working on the project.

[Apollon77]

@karianpour do you do anything?! Else I would prepare a PR for discussion on the weekend ...

[karianpour]

Hi @Apollon77,

I almost finished the refactor. At the step, backward compatibility and extendablity was the goal. I change the code the way that I was sure that no break would happen. The way that you suggest sounds nice, which can be the next step. If you take a look at the code, now you can inject the logic as you suggest with no major code change. Just add a execute function which works as you say. Just executeCommand and get the item , then add the processig logic function to the item. You can also add a listener for processing further actions like ussd support.

[Apollon77]

Ok, then I take the current GitHub code and do some stuff

[Apollon77]

Ahhh ok now I see your changes ... I was iterating on the "npm" code so far ;-) This looks much better. Me personally would have it done differently, but don't matter, it works well that way. So I maybe foxus more on feature stuff. But I will gibe it a try today.

This is a huge improvement!!

[Apollon77]

One question: Is it intended that nearly all comparisms only use "==" instead of "==="? The latter would be more type safe and such ... what are your thoughts on this? (maybe start own issue to discuss such general code style things?)

[zabsalahid]

One question: Is it intended that nearly all comparisms only use "==" instead of "==="? The latter would be more type safe and such ... what are your thoughts on this? (maybe start own issue to discuss such general code style things?)

I agree, coding conventions and guidelines ☕️.

[Apollon77]

Ok, moved to : https://github.com/zabsalahid/serialport-gsm/issues/21

[karianpour]

@zabsalahid I guess that we can close this issue.

[Apollon77]

I will bring in some fixed with my PR today.