Skip to content

Commit

Permalink
Simplified XBee setup connection check
Browse files Browse the repository at this point in the history 
Signed-off-by: Sara Damiano <[email protected]>
  • Loading branch information
@SRGDamia1
SRGDamia1 committed Jun 28, 2023
1 parent 9082d42 commit dc23a83
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 122 deletions.
2 changes: 1 addition & 1 deletion continuous_integration/dependencies.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
{
"name": "TinyGSM",
"owner": "vshymanskyy",
"version": "~0.11.6",
"version": "~0.11.7",
"note": "A small Arduino library for GPRS modules.",
"authors": ["Volodymyr Shymanskyy", "Sara Damiano"],
"frameworks": "arduino",
Expand Down
139 changes: 18 additions & 121 deletions src/modems/DigiXBeeWifi.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,10 +253,10 @@ bool DigiXBeeWifi::extraModemSetup(void) {
GF("SO"), _maintainAssociation ? "40" : "100");
changesMade |= changedSO;
if (changedSO) {
MS_DBG(F("Sleep mode changed to"),
MS_DBG(F("Sleep options changed to"),
_maintainAssociation ? "0x40" : "0x100");
} else {
MS_DEEP_DBG(F("Sleep mode not changed"));
MS_DEEP_DBG(F("Sleep options not changed"));
}

/** Write pin and sleep options to flash and apply them, if needed. */
Expand All @@ -279,10 +279,6 @@ bool DigiXBeeWifi::extraModemSetup(void) {
}

// Set to TCP mode
// NOTE: If
// gsmModem.sendAT(GF("IP"), 1);
// success &= gsmModem.waitResponse() == 1;
// if (!success) { MS_DBG(F("Fail to set IP mode"), success); }
changesMade = false;
bool changedIPMode = gsmModem.changeSettingIfNeeded(GF("IP"), 1);
changesMade |= changedIPMode;
Expand All @@ -294,8 +290,6 @@ bool DigiXBeeWifi::extraModemSetup(void) {


/** Set the socket timeout to 10s (this is default).*/
// gsmModem.sendAT(GF("TM"), 64);
// success &= gsmModem.waitResponse() == 1;
bool changedTM = gsmModem.changeSettingIfNeeded(GF("TM"), "64");
changesMade |= changedTM;
if (changedTM) {
Expand All @@ -305,8 +299,6 @@ bool DigiXBeeWifi::extraModemSetup(void) {
}

/** Set the destination IP to 0 (this is default). */
// gsmModem.sendAT(GF("DL"), GF("0.0.0.0"));
// success &= gsmModem.waitResponse() == 1;
bool changedDL = gsmModem.changeSettingIfNeeded(GF("DL"),
GF("0.0.0.0"));
changesMade |= changedDL;
Expand All @@ -323,123 +315,32 @@ bool DigiXBeeWifi::extraModemSetup(void) {
}

if (success) {
MS_DBG(F("Setup Wifi Network "), _ssid);
MS_DBG(F("Successfully setup Wifi Network"), _ssid);
} else {
MS_DBG(F("Failed Setting WiFi"), _ssid);
}

// Scan for AI last node join request
uint16_t loops = 0;
int16_t ui_db;
uint8_t status;
String ui_op;
bool apRegistered = false;
PRINTOUT(F("Loop=Sec] rx db : Status #Polled Status every 1sec/30sec"));
uint8_t reg_count = 0;
#define TIMER_POLL_AP_STATUS_MSEC 300000
for (unsigned long start = millis(); millis() - start < 300000;
loops++) {
ui_db = 0; // gsmModem.getSignalQuality();
gsmModem.sendAT(GF("AI"));
status = gsmModem.readResponseInt(10000L);
ui_op = String(loops) + "=" + String((float)millis() / 1000) +
"] " + String(ui_db) + ":0x" + String(status, HEX);
if (0 == status) {
ui_op += " Cnt=" + String(reg_count);
#define XBEE_SUCCESS_CNTS 3
if (++reg_count > XBEE_SUCCESS_CNTS) {
PRINTOUT(ui_op);
apRegistered = true;
break;
}
} else {
reg_count =0; //reset
}
PRINTOUT(ui_op);
//Need to pet the watchDog as 8sec timeout ~ but how, LoggerBase::petDog()
delay(1000);
}
if (apRegistered)
{
MS_DBG(F("Get IP number"));
String xbeeRsp;
uint8_t index = 0;
bool AllocatedIpSuccess = false;
// Checkfor IP allocation
#define MDM_IP_STR_MIN_LEN 7
#define MDM_LP_IPMAX 16
for (int mdm_lp = 1; mdm_lp <= MDM_LP_IPMAX; mdm_lp++) {
delay(mdm_lp * 500);
gsmModem.sendAT(F("MY")); // Request IP #
index = gsmModem.waitResponse(1000, xbeeRsp);
MS_DBG(F("mdmIP["), mdm_lp, "/", MDM_LP_IPMAX, F("] '"),
xbeeRsp, "'=", xbeeRsp.length());
if (0 != xbeeRsp.compareTo("0.0.0.0") &&
(xbeeRsp.length() > MDM_IP_STR_MIN_LEN)) {
AllocatedIpSuccess = true;
break;
}
xbeeRsp = "";
}
if (!AllocatedIpSuccess) {
// Since this is the only time we're going to send the credentials,
// confirm that we can connect to the network and get both an IP and DNS
// address.
if (!(gsmModem.isNetworkConnected())) {

This comment has been minimized.

Sign in to view

Copy link
@SRGDamia1

SRGDamia1 Jun 28, 2023

Author Contributor

The isNetworkConnected function in TinyGSM checks AI and MY and I just updated it to check NS as well.

This comment has been minimized.

Sign in to view

Copy link
@neilh10

neilh10 Jun 28, 2023

Contributor

I think you are making it impossible to debug. There are expected sequences that need to be visible.
You are setting it up for a "hope and prayer" testing.
if (!gsmModem.waitForNetwork()) {

This comment has been minimized.

Sign in to view

Copy link
@SRGDamia1

SRGDamia1 Jun 28, 2023

Author Contributor

Wait for network waits for AI to read 0 and then for MY and NS to be valid.
PRINTOUT(
F("XbeeWiFi not received IP# -hope it works next time"));
// delay(1000);
// NVIC_SystemReset();
F("... Initial WiFi connection failed - resetting module"));
loggerModem::modemHardReset();
delay(50);
success = false;
} else {
// success &= AllocatedIpSuccess;
PRINTOUT(F("XbeeWiFi IP# ["), xbeeRsp, F("]"));
xbeeRsp = "";
// Display DNS allocation
bool DnsIpSuccess = false;
#define MDM_LP_DNSMAX 11
for (int mdm_lp = 1; mdm_lp <= MDM_LP_DNSMAX; mdm_lp++) {
delay(mdm_lp * 500);
gsmModem.sendAT(F("NS")); // Request DNS #
index &= gsmModem.waitResponse(1000, xbeeRsp);
MS_DBG(F("mdmDNS["), mdm_lp, "/", MDM_LP_DNSMAX, F("] '"),
xbeeRsp, "'");
if (0 != xbeeRsp.compareTo("0.0.0.0") &&
(xbeeRsp.length() > MDM_IP_STR_MIN_LEN)) {
DnsIpSuccess = true;
break;
}
xbeeRsp = "";
}

if (false == DnsIpSuccess) {
success = false;
PRINTOUT(F(
"XbeeWifi init test FAILED - hope it works next time"));
} else {
PRINTOUT(F("XbeeWifi init test PASSED"));
}
PRINTOUT(F("... Initial WiFi connection succeeded!"));
success = true;
}
#if 0 // defined MS_DIGIXBEEWIFI_DEBUG
// as of 0.23.15 the modem as sensor has problems
int16_t rssi, percent;
getModemSignalQuality(rssi, percent);
MS_DBG(F("mdmSQ["),toAscii(rssi),F(","),percent,F("%]"));
#endif // MS_DIGIXBEEWIFI_DEBUG
gsmModem.exitCommand();
}
else
{ // !apRegistered could be invalid SSID, no SSID, or stuck module
PRINTOUT(F(
"XbeeWiFi AP not Registered - reseting module, hope it works "
"next time"));
loggerModem::modemHardReset();
delay(50);
// NVIC_SystemReset();
success = false;
} else {
PRINTOUT(F("... Initial WiFi connection succeeded!"));
success = true;
}
} else {
success = false;
gsmModem.exitCommand();
}

if (false == success) { PRINTOUT(F("Xbee '"), _modemName, F("' failed.")); }

return success;
}

Expand All @@ -451,7 +352,7 @@ void DigiXBeeWifi::disconnectInternet(void) {
String oldRemoteIp = gsmClient.remoteIP();
IPAddress newHostIp = IPAddress(127, 0, 0, 1); // localhost
gsmClient.connect(newHostIp, 80);
MS_DBG(gsmModem.getBeeName(), oldRemoteIp, F(" disconnectInternet set to "),
MS_DBG(gsmModem.getBeeName(), oldRemoteIp, F("disconnectInternet set to"),
gsmClient.remoteIP());

gsmModem.restart();
Expand Down Expand Up @@ -719,7 +620,3 @@ String DigiXBeeWifi::getWiFiId(void) {
String DigiXBeeWifi::getWiFiPwd(void) {
return _pwd;
}
//If needed can provide specific information
//String DigiXBeeWifi::getModemDevId(void) {return "DigiXbeeWiFiId";}


4 comments on commit dc23a83

@SRGDamia1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neilh10, I wanted to use the functions in TinyGSM partly to simplify the code, and partly because the "AI" check in TinyGSM already has logic in it to reset the XBee if it gets stuck in some of it's most common sticking points.

@neilh10
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SRGDamia1 I think you are making it impossible to debug. There are expected sequences that need to be visible.
You are setting it up for a "hope and prayer" testing.
If there is a problem the trace needs to be visible as to why the program took the path it took.
I agree with the cleanup - however you have said in the past you can't thoroughly test it, so set it up that other people can follow the program path, including what is coming back from the module.

@SRGDamia1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did decrease the number of printouts. I wanted to remove the repeat of the logic already in TinyGSM. It made more sense to me to rely on watching the AT commands with streamDebugger rather than re-printing the AT result for the user.

@neilh10
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok .. I come back to my item how can knowledgeable uses be all on the same page .. #435
What is the test setup that we can all use for discussing of the same code. AND what is thorough testing for real world edge use cases
You may be a savant for watching the AT trail, but the next layer of code path monitoring is missing . Please make it debugable for us mere mortals, and a platform where the logic can be discussed.
I've got even more debug in my fork of TinyGSM to be able to monitor the different layers of TinyGSM calls.
IMHO as a user with too many years using ModularSensors (and I'm an advocate) it is orientated toward the "Shop Window" and is hard to debug. I need to fork to build in ways of understand the code which then is a long time before there is any discussion about how experience gets back into the main code base
Just my 2cents from a mortal using the code.

Please sign in to comment.