Improved GSMSSHIELD library

You can now use the classes like this:

CSIMCOM900 gsm(&Serial1);
CCallGSM call(gsm);
CSMSGSM sms(gsm);

I have removed the hard coding that locked you into using SoftwareSerial with Uno and Serial1 with Mega.

Also enclosed all the literal strings with F("") and added some additional functions to take flash string parameters as well as regular string parameters. So the library uses much less global memory.

The GSM_GPRSLibrary_Call.ino example still seems to initialise my shield successfully but I have not done any testing beyond that.

And I can’t seem to find the original GSMSHIELD library on Github.

Any suggestions as to where is the most appropriate place to put it?

Create my own repository for it?

Try and get one of the other versions to pull in my changes?

GSMSHIELD.zip (45.9 KB)

There's a link in the readme to a Google Code page:
https://code.google.com/archive/p/gsm-shield-arduino/
That page links to a GitHub repository (Google Code shut down a while ago):

So I think that must be the primary repository.

I think it's a good idea to try to get your improvements merged into the main repository. Having a lot of forks of a library, each with their own unique improvements is frustrating because it's hard to know which one to use and it's more administrative effort overall for all the various owners to maintain (or more likely not maintain) their individual forks. If you can get your changes merged to the main repo then you hand off that ongoing work.

Of course the first step to submitting a pull request is to make a fork so actually you will be publishing your version of the library. If the owner of the main repo is taking a while to merge the PR people can always just download your fork. The links from the main repository to your fork will make it easier for others to find your fork.

Done: https://github.com/boylesg/GSM-GPRS-GPS-Shield/commit/9aec1e22b28e93c330301e99510cfbc70c3102d4

Also changed the code to use Hungarian notation.

Sorry for those who hate Hungarian notation, but I find it more intuitive to read.

Also changed int to int16_t and long unsigned to uint32_t etc - I reckon those data types are 'cleaner' than the regular C ones.

Improved indenting to make the if statement etc easier to grasp and generally tidied up the code.

Removed deprecated functions and commented out (and presumably no longer required) code.

I tried the SMS example sketch and seemed to successfully send a text message to my phone....assuming the SIM card I put in the device is OK.

It’s better to do atomic commits:

In this case I would have made the following individual commits:

  • Remove hard coding that limits you to using SoftwareSerial for Uno and Serial1 for Mega
  • Wrap all literal strings in F()
  • Add function versions that take flash strings
  • Use Hungarian notation
  • Use stdint types
  • Improve indentation
  • Remove deprecated functions
  • Comment out (presumably) no longer required code

That makes it much easier to track the changes that were made (including descriptive commit messages), makes it possible to roll back through the changes for troubleshooting, and makes it easy to submit pull requests for each individual change.

What happens if the author of the library is interested in the serial interface hardcoding but is absolutely against you going through and renaming all the variables? If those changes were each made in a separate commit and each commit was submitted as a separate pull request then they can easily take one but not the other. Even if they do happen to agree with every single one of your changes it’s very difficult to review the pull request because there are so many unrelated changes mashed together, which makes it a lot of work to check whether any bugs were introduced.

pert:
It's better to do atomic commits:
Developer Tip: Keep Your Commits "Atomic" - Fresh Consulting
In this case I would have made the following individual commits:

  • Remove hard coding that limits you to using SoftwareSerial for Uno and Serial1 for Mega
  • Wrap all literal strings in F()
  • Add function versions that take flash strings
  • Use Hungarian notation
  • Use stdint types
  • Improve indentation
  • Remove deprecated functions
  • Comment out (presumably) no longer required code

That makes it much easier to track the changes that were made (including descriptive commit messages), makes it possible to roll back through the changes for troubleshooting, and makes it easy to submit pull requests for each individual change.

What happens if the author of the library is interested in the serial interface hardcoding but is absolutely against you going through and renaming all the variables? If those changes were each made in a separate commit and each commit was submitted as a separate pull request then they can easily take one but not the other. Even if they do happen to agree with every single one of your changes it's very difficult to review the pull request because there are so many unrelated changes mashed together, which makes it a lot of work to check whether any bugs were introduced.

Well I guess to author can just make the same changes I made minus the hungarian notation.....no skin off my nose.

If I find anything else not working properly I will work on it, but I prefer Hungarian notation because it makes it easier to read and grasp.

Probably should add into my commit and pull comments what the key changes were to remove the hard coded SoftwareSerial/Serial1

Can you edit Github commit and pull comments?

If so how?

I give up, I just added a new commit to change the comment:


This upload was simply to change the commit comment because I have no idea how to edit the comment of an existing commit.

Removed hard coding that limits you to using SoftwareSerial for Uno and Serial1 for Mega

You can now use the classes like this:

CSIMCOM900 gsm(&Serial1);
SoftwareSerial ss(8, 9);
CSIMCOM900 gsm(&Serial1);
//CSIMCOM900 gsm(&ss);
CSMSGSM sms(gsm);

void setup()
{
gsm.begin(115200);
}

Also wrapped all the literal strings in F() and added appropriate function versions to take flash strings rather than normal c strings.

Changed class, data members and variable to use Hungarian notation.

Removed commented out, and presumably no longer needed, code.

Removed deprecated function and changed example sketch accordingly.

Cleaned up code indenting to make the code easier to read.

Changed WideTextFinder to CSWSerial so as to be consistent with CHWSerial (formerly HWSerial).

If you don't want all the cosmetic code changes then the key changes to removed the hard coding are as follows:

GSMBase.h/cpp
class CGSMBase
{
public:
CGSMBase(CSIMCOM900& rSimComm900): m_rSimComm900(rSimComm900){};

protected:
CSIMCOM900& m_rSimComm900;
};

SMS.h/cpp, InetGSM.h/cpp, GPSGSM.h.cpp, call.h/cpp

class CSMSGSM: protected CGSMBase
{
public:
CSMSGSM(CSIMCOM900& rSimComm900): CGSMBase(rSimComm900){};

class CGSM
{
public:

// Constructors
CGSM(const SoftwareSerial pStream): m_nStatus(IDLE)
{
m_pStream = (Stream
)pStream;
m_nSizeofStream = sizeof *pStream;
}

CGSM(const HardwareSerial pStream): m_nStatus(IDLE)
{
m_pStream = (Stream
)pStream;
m_nSizeofStream = sizeof *pStream;
}
.
.
.
.
.
protected:
// Constructors
CGSM(); // Don't want this constructor to be used

void beginSerial(const uint32_t nBaud)
{
if (m_nSizeofStream == sizeof(HardwareSerial))
{
((HardwareSerial*)m_pStream)->begin(nBaud);
}
else if (m_nSizeofStream == sizeof(SoftwareSerial))
{
((SoftwareSerial*)m_pStream)->begin(nBaud);
}
}
bool isUsingSS()
{
return m_nSizeofStream == sizeof(SoftwareSerial);
}

bool isUsingHS()
{
return m_nSizeofStream == sizeof(HardwareSerial);
}

// Serial source used to communicate with GSM
Stream* m_pStream; // Either a SoftwareSerial or a HardwareSerial object passed in through the constructor.
uint16_t m_nSizeofStream; // Size of the the SoftwareSerial or a HardwareSerial object so we can tell which type was used.

HWSerial.h/cpp, SWSerial.h/cpp

These are now created as local objects so that the default empty constructor is not used.

class CHWSerial
{
private:
//Don't want this constructor to be used
CHWSerial(): _rSerial(Serial)
{
};

public:
CHWSerial(HardwareSerial& rSerial);

class CSWSerial
{
private:
SoftwareSerial& m_rSoftwareSerial;

//Don't want this constructor to be used
CSWSerial(): _rSerial(Serial)
{
};

public:
// Constructor:
CSWSerial(SoftwareSerial &rStream, int16_t nTimeout = 5); // Ethernet constructor

SIM900.h/cpp

class CSIMCOM900 : public virtual CGSM
{
protected:
CSIMCOM900(): CGSM(NULL)(); // Don't want this constructor to be used

public:
CSIMCOM900(const SoftwareSerial *pStream): CGSM(pStream){};
CSIMCOM900(const HardwareSerial *pStream): : CGSM(pStream){};

You can't edit commits on GitHub but if you clone the repository to your computer you can use either a Git client program or just Git itself from the command line to do any edits you like. I like to use the interactive rebase feature. It makes it easy to edit commits, commit messages, reorder commits, combine commits, etc:
https://git-scm.com/docs/git-rebase
So the command would be:

git rebase -i {commit hash before the earliest commit you want to edit}

That will open a text file that has a list of the commits with a "pick" command before each. There is a comment that lists the other commands you can change "pick" to, or you can rearrange the list of commits. After you're ready to proceed you only need to close the text file and save it. Depending on the commands you run, other text files may open during the rebase with additional options.

I used to use the GitHub Desktop Git client and it was pretty good but as I got more advanced with Git I found it too limiting. I tried out all the other popular client software that ran on Windows and found that Git Extensions was by far my favorite:

I've started using the Git command line more lately but for the usual Git tasks I find a GUI is still faster. You can do an interactive rebase in Git Extensions by right clicking on the commit before the earliest one you want to edit, then selecting "Rebase current branch on". Then check the "Interactive Rebase" box in the rebase dialog before clicking "Rebase". From there the process is the same as when doing an interactive rebase from the command line.

After editing a commit you need to do a force push back to GitHub since that will make the history of your local clone different from the history in your GitHub fork. It's a bad idea to change the history of a repository when other people are relying on it not changing but when you are just working in a feature branch for the purpose of a pull request it's fine and actually encouraged because often after review and discussion some changes to the proposal are necessary. It's usually a good idea to leave a comment on the pull request thread after a force push that makes any significant changes, especially after the PR has already been reviewed.

You can edit Pull Request messages on GitHub by simply clicking the pencil icon on the top right of your comment.

pert:
You can’t edit commits on GitHub but if you clone the repository to your computer you can use either a Git client program or just Git itself from the command line to do any edits you like. I like to use the interactive rebase feature. It makes it easy to edit commits, commit messages, reorder commits, combine commits, etc:
Git - git-rebase Documentation
So the command would be:

git rebase -i {commit hash before the earliest commit you want to edit}

That will open a text file that has a list of the commits with a “pick” command before each. There is a comment that lists the other commands you can change “pick” to, or you can rearrange the list of commits. After you’re ready to proceed you only need to close the text file and save it. Depending on the commands you run, other text files may open during the rebase with additional options.

I used to use the GitHub Desktop Git client and it was pretty good but as I got more advanced with Git I found it too limiting. I tried out all the other popular client software that ran on Windows and found that Git Extensions was by far my favorite:
GitHub - gitextensions/gitextensions: Git Extensions is a standalone UI tool for managing git repositories. It also integrates with Windows Explorer and Microsoft Visual Studio (2015/2017/2019).
I’ve started using the Git command line more lately but for the usual Git tasks I find a GUI is still faster. You can do an interactive rebase in Git Extensions by right clicking on the commit before the earliest one you want to edit, then selecting “Rebase current branch on”. Then check the “Interactive Rebase” box in the rebase dialog before clicking “Rebase”. From there the process is the same as when doing an interactive rebase from the command line.

After editing a commit you need to do a force push back to GitHub since that will make the history of your local clone different from the history in your GitHub fork. It’s a bad idea to change the history of a repository when other people are relying on it not changing but when you are just working in a feature branch for the purpose of a pull request it’s fine and actually encouraged because often after review and discussion some changes to the proposal are necessary. It’s usually a good idea to leave a comment on the pull request thread after a force push that makes any significant changes, especially after the PR has already been reviewed.

You can edit Pull Request messages on GitHub by simply clicking the pencil icon on the top right of your comment.

I have played with GIT command line a bit. But it is a bloody pain compared to point and click and drag and drop.

This is ripe for a more sophisticated Windows GUI that exposes more of the GIT command line stuff.

There are already several of those GUIs. As I said, my preference is Git Extensions. The only processes I can't do with the Git Extensions GUI are fairly uncommon, like working with subtrees.

GitHub desktop is a reasonable choice for someone new to working with Git repositories on their computer who wants a very gentle learning curve and doesn't need to do any but the most basic processes (branch, commit, push, pull). The commit interface of Git Desktop is actually excellent. I didn't find any other software that made it so easy to stage specific lines of a diff for a commit. Git Extensions was the only thing I found that even came close to it.