Skip to content
This repository was archived by the owner on Jun 5, 2019. It is now read-only.

Conversation

@dale-lyons
Copy link

When restarting the device Thread.Yield
was being used. This caused the attempts to
connect to complete too quickly. A simple
Thread.Sleep(10) fixes the problem.

When restarting the device Thread.Yield was being used. This caused the attempts to connect to complete too quickly. A simple Thread.Sleep(10) fixes the problem.
@smaillet-ms
Copy link
Member

Injecting arbitrary thread sleeps is never a valid solution - the yields were a fix for the previous abuse of sleeps.

Please file a separate issue including detailed reproducible steps so we can solve the problem in a more appropriate way.

@doingnz
Copy link
Contributor

doingnz commented Jun 10, 2016

I have also struggled with the reconnect to the Debugger problem(s). The current implementation is fragile and easily broken with the wrong Sleep(n) added or removed. It would be good to devise a more robust protocol to sync the embedded system and the PC.

I have specifically been looking at running the unit tests. The tests themselves run fine for the most part, but I found that the PC and FW portions of the debugger would not re-sync correctly at the start or end of a test, resulting in a test incorrectly marked as failed.

I have found that changing the hard coded magic values used for timeout and retries in Test\Platform\Tools\MFTestSystem\Harness.cs for TryToConnect(...) made a big difference.

i.e. was

 connected = m_engine.TryToConnect(200, 500, true, ConnectionSource.TinyCLR); 

much better if I change to

// define at top of class private const int DEFAULT_DEVICE_TIMEOUT_MS = 700; private const int DEFAULT_DEVICE_SHORT_RETRY_COUNT = 10; private const int DEFAULT_DEVICE_LONG_RETRY_COUNT = 300; ... connected = m_engine.TryToConnect(DEFAULT_DEVICE_LONG_RETRY_COUNT, DEFAULT_DEVICE_TIMEOUT_MS, true, ConnectionSource.TinyCLR); 

There were some other places where the timeout was 500, and I used the same DEFAULT_DEVICE_TIMEOUT_MS const there too. Also labelled one spot where the retries are only 10.

With these changes the unit tests run perfectly on my ARM920T system and they still run correctly on the STM32 Discovery solution included with NetMF.

@doingnz
Copy link
Contributor

doingnz commented Jun 10, 2016

to be clear, I have reverted the suggested Sleep(10) edits back to Yield in my working system

@smaillet-ms
Copy link
Member

The implementation of the wire protocol code base is something I've generally despised for a long while now. It is one of the oldest pieces of the desktop managed code base and due to it's impact when it doesn't work - it's always been one of those "ain't broke - don't fix" things. I don't think there's much worth salvaging in the current implementation as so much of the runtime and libraries has changed (e.g. streams have async support built in and the language has async/await to make the async nature of the whole thing simpler.

The device side code is mostly salvageable though it, and much else in NETMF, could get help from C++ co-routines once they are finally ratified into the language.

That said, if all you did was tweak the timeout and retry values and provided them as symbolic constants and it works on a couple devices, go ahead with a PR, I'll try against the MCB board and a couple of boards from GHI and Secret Labs.

@smaillet-ms smaillet-ms closed this Jul 9, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

3 participants