Sound processing doesn't start when DISABLE_AUTO_SWITCH_TO_AP is not set and can't connect to AP

[acoulson2000]

I believe I found a bug that occurs when DISABLE_AUTO_SWITCH_TO_AP is NOT set - so we DO want to fall back to SOFT_AP mode if we can't connect to the configured AP. Since print_ip get's set to 0, sound processing and ws2812 control never starts.

//// This section:

		wifi_station_connect();
		printf("\n");
		printed_ip = 0;

////Needs to be before:

#ifndef DISABLE_AUTO_SWITCH_TO_AP
#define MAX_CONNECT_FAILURES_BEFORE_SOFTAP 2
#ifdef MAX_CONNECT_FAILURES_BEFORE_SOFTAP
if( wifi_fail_connects > MAX_CONNECT_FAILURES_BEFORE_SOFTAP )
{
GoAP( 0 ); // giving up on station so start ADC so need ExitCritical() which is done in GoAP
CSConnectionChange();
wifi_fail_connects = 0;
///// And printed_ip needs to be set:
printed_ip = 1;
}
#endif
#endif

[cnlohr]

Just pinging @AEFeinstein for confirmation to double-check this... I don't know if we've ever tested the fallback to soft ap feature on the new branch. The code change looks a little strange to me. I believe @acoulson2000 is referring to commonservices.c:993

Also @acoulson2000 if printed_ip is set to 1 here, then, won't it just get unset in the lines immediately following?

Could you chop out a little more before and after?

[acoulson2000]

Oop.. sorry, I should have mentioned it's in commonservices.c. Yes, pasting a bigger chunk, whcih seems to work for me, below. If I wasn't such a git neophyte, I could probably figure out a way to put it on a branch and do a pull request :-/

		...
	if( opm == 1 ) {
		struct station_config wcfg;
		struct ip_info ipi;
		int stat = wifi_station_get_connect_status();

		if( stat == STATION_WRONG_PASSWORD || stat == STATION_NO_AP_FOUND || stat == STATION_CONNECT_FAIL ) {
			wifi_station_disconnect();
			wifi_fail_connects++;
			printf( "Connection failed with code %d... Retrying, try: %d\n", stat, wifi_fail_connects );
			wifi_station_connect();
			printf("\n");
			printed_ip = 0;
#ifndef DISABLE_AUTO_SWITCH_TO_AP
#define MAX_CONNECT_FAILURES_BEFORE_SOFTAP 2
#ifdef MAX_CONNECT_FAILURES_BEFORE_SOFTAP
			if( wifi_fail_connects > MAX_CONNECT_FAILURES_BEFORE_SOFTAP )
			{
				GoAP( 0 ); // giving up on station so start ADC so need ExitCritical() which is done in GoAP
				CSConnectionChange();
				wifi_fail_connects = 0;
				printed_ip = 1;
			}
#endif
#endif
		} else if( stat == STATION_GOT_IP && !printed_ip ) {
		...

[AEFeinstein]

@cnlohr I did not test any of this in https://github.com/cnlohr/swadge2019 b/c it never uses station mode.

@acoulson2000 nothing wrong with being a neophyte. GitHub does have some help when it comes to this sort of thing:
https://help.github.com/articles/creating-and-deleting-branches-within-your-repository/
https://help.github.com/articles/about-pull-requests/
As for the actual change, I don't quite understand why your fix works. I suspect it has to something to do with this from timerFunc100ms(), but I'm no colorchord expert.

 else if( hpa_is_paused_for_wifi && printed_ip )
 {
     StartHPATimer(); // Init the high speed ADC timer.
     hpa_is_paused_for_wifi = 0; // only need to do once prevents unstable ADC
 }

It does feel weird to call wifi_station_connect() and then immediately call GoAP().

[cnlohr]

hhnnggg I won't be able to test this for almost two weeks :( If there is any way you can chase this down and make it a little cleaner maybe call the underlying functions? Or maybe we need a facility in esp82xx to say "enable/disable timing critical stuff"?

[acoulson2000]

@AEFeinstein I'm pretty sure that is the problem (although in the main branch, the function is myTimer() )

@cnlohr The issue seems to be the reverse of what you mention - in the current code, if ( wifi_fail_connects > MAX_CONNECT_FAILURES_BEFORE_SOFTAP ), GoAP() gets called and printed_ip is set to 1, but then immediately afterward, printed_ip is set back to 0, so StartHPATimer() never gets called on the next tick of myTimer()

[acoulson2000]

And yeah, the use of the printed_ip flag to sort of reflect the state of whether the HPATimer needs to be start off in another are of code was kind of confusing and hard to track down (although my unfamiliarity with the code is the bigger problem :-)

[cnlohr]

Not sure... does that mean all is well?