Stopping the loop() task execution when using mDashShadowUpdate()

[cesanta]

Stopping the loop() task execution when using mDashShadowUpdate()

[ssbrewco]

Side note: If possible, bring back the CLI as well. Not having it breaks the device provisioning flow we had.

Issue #20 open on this one.

[younesmaia]

I can confirm the same problem is happening to me on mDash v1.2.16 and Platformio ESP32 core 6.3.1 (latest).
Opened a ticket about it:
#21

The loop() task indeed stops when mDash functions are called. I'm having random issues even with mDashConfigGet.

Side note: If possible, bring back the CLI as well. Not having it breaks the device provisioning flow we had.

[cpq]

Gents, the code is wide open.

Sends your PRs.

[younesmaia]

I've temporarily commented lines 104 and 107 (xSemaphoreTakeRecursive and xSemaphoreGiveRecursive) for testing and mDash works perfectly (for as long as you don't call any mDash function while another is still running 😀).
So, this is definetely an Mutex-related issue.
But, I could not find anything wrong with the implementation in mDash.c itself. The calls for taking and giving the semaphore are properly placed and there should be no holdup...

[Thermelgy-Repo]

I have solved the issue, plz check the PR. #24

[cpq]

You just added a duplicate of notify without the locks - that's not a proper fix.

I guess that the core of the issue is here:

mDash/src/mDash.c

Lines 736 to 740 in 8bd6647

	void mDashPoll(void) {
	MDashMutexLock();
	mg_mgr_poll(&s_mgr, 50);
	MDashMutexUnlock();
	}

All functions that may be called by the mDashPoll(), for example mDashShadowUpdate(), will deadlock.
So I guess that the proper fix would be to just remove the locks from mDashShadowUpdate() and document that the mutex locks must be called manually before and after, in case the function is called outside the mdash thread.

[Thermelgy-Repo]

You just added a duplicate of notify without the locks - that's not a proper fix.

I guess that the core of the issue is here:

mDash/src/mDash.c

Lines 736 to 740 in 8bd6647

	void mDashPoll(void) {
	MDashMutexLock();
	mg_mgr_poll(&s_mgr, 50);
	MDashMutexUnlock();
	}

All functions that may be called by the mDashPoll(), for example mDashShadowUpdate(), will deadlock. So I guess that the proper fix would be to just remove the locks from mDashShadowUpdate() and document that the mutex locks must be called manually before and after, in case the function is called outside the mdash thread.

Thankq @cpq for your suggestion, So you meant to directly remove the MutexLocks from the mDashNotify() as well as mDashPoll() functions?

[cpq]

@Thermelgy-Repo from the mDashNotify only.
It would be a user responsibility to surround the mDashNotify call with locks manually if it gets called from the outside of Mongoose task.