Rename OFF constant

[jgartner1]

In the file GSM.h you find the following enum on line 25:

enum GSM3_NetworkStatus_t { ERROR, IDLE, CONNECTING, GSM_READY, GPRS_READY, TRANSPARENT_CONNECTED, OFF};

But the constant OFF at the end has caused me some issues with other libaries because they also used this very general name but with a different value. So I would suggest to replace this line with the following:

enum GSM3_NetworkStatus_t { GSM_ERROR, GSM_IDLE, GSM_CONNECTING, GSM_READY, GPRS_READY, TRANSPARENT_CONNECTED, GSM_OFF};

With this simple renaming I got my project running, and hadn't any double-definitions. I know that this would be a problem if someone updates their libary and wants to access a constant that doesn't exist anymore. For this you could add the old line as a comment and tell them that this deprecated version could also be used.

[FrancMunoz]

I've noticed that this constant is never used, so i'd like to propose this changes in GSM.cpp, following the name suggested by @jgartner1 :

bool GSM::shutdown()
{
  MODEM.send("AT+CPWROFF");

  if (MODEM.waitForResponse(40000) == 1) {
    MODEM.end();
    _state = GSM_OFF;
    return true;
  }

  return false;
}

bool GSM::secureShutdown()
{
  MODEM.end();
  _state = GSM_OFF;
  return true;
}

If not it should be removed instead of renamed.

[sandeepmistry]

I'm ok with renaming OFF to GSM_OFF, however this would break compatibility with the original GSM library. However, adding a GSM_ prefix to the other constants is not something I'm keen on.

@cmaglie @tigoe what are your thoughts on the topic?

[tigoe]

I think if OFF has a global value that is useful to other libraries (like, say 0), then it's good to keep it as such. But if OFF means something specific to the GSM library, then it should be GSM_OFF.

t.

On Wed, Sep 12, 2018 at 3:12 PM, Sandeep Mistry notifications@github.com wrote:

I'm ok with renaming OFF to GSMOFF, however this would break compatibility with the original GSM library. However, adding a GSM prefix to the other constants is not something I'm keen on.

@cmaglie https://github.com/cmaglie @tigoe https://github.com/tigoe what are your thoughts on the topic?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/arduino-libraries/MKRGSM/issues/39#issuecomment-420764718, or mute the thread https://github.com/notifications/unsubscribe-auth/AAXOn1gmD067rvRnjMqp5Xb-Y9MY7yjZks5uaVy7gaJpZM4V5GZw .

[sandeepmistry]

Ok, so OFF has a value of 6 now. So makes sense to rename it to GSM_OFF.

@jgartner1 or @FrancMunoz are you ok with renaming OFF in the library to GSM_OFF, but leave the other enums alone for now? (bonus points for opening a pull request for this).

[FrancMunoz]

What do you think about using the constant, I think it's never used.

I've noticed that this constant is never used, so i'd like to propose this changes in GSM.cpp, following the name suggested by @jgartner1 :

bool GSM::shutdown()
{
  MODEM.send("AT+CPWROFF");

  if (MODEM.waitForResponse(40000) == 1) {
    MODEM.end();
    _state = GSM_OFF;
    return true;
  }

  return false;
}

bool GSM::secureShutdown()
{
  MODEM.end();
  _state = GSM_OFF;
  return true;
}

[FrancMunoz]

Done in #48 ... lol for bonus points...

[sandeepmistry]

Thanks! Closed via #44.