Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix reader thread code to fit the CC protocol #19

Closed
eiaro opened this issue Oct 21, 2018 · 16 comments
Closed

Fix reader thread code to fit the CC protocol #19

eiaro opened this issue Oct 21, 2018 · 16 comments

Comments

@eiaro
Copy link

eiaro commented Oct 21, 2018

This code need to be fixed so the CC protocol is implemented correct. My assumptions when writing it was more than slightly off, but should be a good basis.

  • Reading and parsing

    private async void ReadSerialPort(UnifiedNetworkProcessorInterface port)
    {
    if (port == null) throw new ArgumentNullException(nameof(port));
    while (true)
    try
    {
    var serialPacket = await SerialPacket.ReadAsync(port.InputStream).ConfigureAwait(false);
    // SerialPacket.ReadAsync() return the correct package class, so
    // we can start processing them into the correct queue here
    if (serialPacket is SynchronousResponse)
    {
    responseQueue.Add(serialPacket);
    continue;
    }
    if (serialPacket is AsynchronousRequest) eventQueue.Add(serialPacket);
    // Ok, not sure but if we reach here something is afoot?
    }
    catch (Exception e)
    {
    // TODO improve this by adding a good error handling
    OnError(e);
    }
    }

  • Sending and waiting for response

    public Task<byte[]> Send(SubSystem subSystem, CancellationToken cancellationToken, params byte[] payload)
    {
    return Send(subSystem, payload,
    msg => { return msg is SynchronousResponse && msg.SubSystem == subSystem; },
    cancellationToken);
    }

  • Code does now allow the Cmd1

    private Task<byte[]> Send(SubSystem subSystem, byte[] payload, Func<SynchronousResponse, bool> predicate,
    CancellationToken cancellationToken)
    {
    return RetryAsync(async () =>
    {
    var request = new SynchronousRequest(subSystem, 0, payload);
    transmitQueue.Add(request);
    var response = await WaitForResponse(msg => predicate((SynchronousResponse) msg), cancellationToken)
    .ConfigureAwait(false);
    return ((SynchronousResponse) response).Payload;
    }, $"{subSystem} {(payload != null ? BitConverter.ToString(payload) : string.Empty)}", cancellationToken);
    }

@Mr-Markus
Copy link
Owner

I have some ideas and already tried to implement it in a local branch. I will further work on it and update the status in this Issue

@eiaro
Copy link
Author

eiaro commented Oct 21, 2018

Nice. I think if the Read part is corrected with your knowledge of the protocol, we would jump light years ahead the next days.

I still struggle getting my CC2531 working. When I use a old ZNP hex I get a warning in Z Tool, but with the current ZNP hex it dosn't find my device at all.

@Mr-Markus
Copy link
Owner

I think so, too

Which Z-Stack version so you use? 3.0.2 is the latest. And you have to execute about 3 or 4 commands to find your device.

If you could explain the steps you are doing in Z-Tool I can help you.

@eiaro
Copy link
Author

eiaro commented Oct 21, 2018

I get it working using Z-Stack 3.0.1. 3.0.2 does not work (so far).

@Mr-Markus
Copy link
Owner

I also used 3.0.1 and it worked with it.
Today I wanted to update it to the latest version, but there was no time for it. Luckily :-)

There is already another Cc2531 stick on its way to me. Then I would wait for it and flash latest firmware on that stick, so that I do not break my current hardware.

@eiaro
Copy link
Author

eiaro commented Oct 21, 2018

To me this looks like SREQ is the command and SRSP is the response, and AREQ is messages sent that isn't expected replies for the "user" sent from devices, nor does it expect a reply if sent from "user".

I don't see any commands where you send a SREQ and receives a SRSP, and then need to wait for AREQ...?

@Mr-Markus
Copy link
Owner

Following from Z-Stack docs:

  • 0: POLL. A POLL command is used to retrieve queued data. This command is only applicable to SPI transport. For a POLL command the subsystem and Id are set to zero and data length is zero.
  • 1: SREQ: A synchronous request that requires an immediate response. For example, a function call with a return value would use an SREQ command.
  • 2: AREQ: An asynchronous request. For example, a callback event or a function call with no return value would use an AREQ command. 
  • 3: SRSP: A synchronous response. This type of command is only sent in response to a SREQ command. For an SRSP command the subsystem and Id are set to the same values as the corresponding SREQ. The length of an SRSP is generally nonzero, so an SRSP with length=0 can be used to indicate an error.

So if i understanding it corrext an AREQ can be a callback function. And this mapping between an SREQ and an AREQ callback is done in zdo_ReqRspMap.json

@eiaro
Copy link
Author

eiaro commented Oct 22, 2018

Yes, but not the kind of callback you would wait for. Say you send a discovery to see what nodes is on, when would you return to the user? On the first node reply, or until you time out waiting for a unknown number of callbacks?

@Mr-Markus
Copy link
Owner

Ok i understand. And how do you think we should handle this?

One explicit example is the ZDO_SIMPLE_DESC_REQ which has an callback with ZDO_NODE_DESC_RSP.
So if an SREQ is send it awaits an SRSP and also have an Event with that AREQ callback.

So... is it can be correct to queue it in the eventQueue like other AREQ. And the EventBridge class handles it (e.g. find device in the device list or database and enrich it's data)

@eiaro
Copy link
Author

eiaro commented Oct 22, 2018

There should be an event thread running to enumerate the node info. It should also handle any subscription to nodes.

@Mr-Markus
Copy link
Owner

Maybe one event thread per device / node^^

@eiaro
Copy link
Author

eiaro commented Oct 22, 2018

Sounds like a lot of work getting thread safe to no real advantage.

@Mr-Markus
Copy link
Owner

Yes, lot of work. But what if e.g. 100 devices are sending messages on network startup at the same time?
Then processing AREQ Messages can improve usability.

But this could be an improvment after first stable version...

@eiaro
Copy link
Author

eiaro commented Oct 22, 2018

I'm sure 1 thread can manage that. Those 100 threads would block the queue, and the feeder thread would be unable to input data and you would easily get a dead lock.

@Mr-Markus
Copy link
Owner

And what did you mean with:

It should also handle any subscription to nodes.

@eiaro
Copy link
Author

eiaro commented Oct 22, 2018

var ep = node.GetEndpoint<ZCLOnOff>[0]();
ep.Changed += (_,e) => DoSomething(e);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants