jefffhaynes / XBee

A .NET library for XBee wireless controllers
MIT License
40 stars 17 forks source link

throwing InvalidOperationException in Read() and Frame documentation discrepancies #62

Closed andygikling closed 4 years ago

andygikling commented 4 years ago

On UWP, with the demo app, I call discover nodes and end up with an erroneous exception from SerialDeviceStream.cs. (I'm using a 900HP radio in API mode with escapes).

public override int Read(byte[] buffer, int offset, int count)
{
     throw new InvalidOperationException();
}

Anyone know how on earth am I getting here? I can't find the code path which calls this method - there isn't one in this repo... Seems like an Async bug in the BinarySerializer library.

The console output looks like this - many of the pieces of the message are coming in correctly:

D-End: Value (65534) @ 10 D-End: ShortAddress (FFFE) @ 10 D-Start: LongAddress @ 10 D-Start: Value @ 10 Rx Trace - 0013a20041828c51 D-End: Value (5526146540276817) @ 18 D-End: LongAddress (0013A20041828C51) @ 18 D-Start: ReceivedSignalStrengthIndicator @ 18 D-Start: SignalLoss @ 18 Rx Trace - 50 D-End: SignalLoss (80) @ 19 D-End: ReceivedSignalStrengthIndicator (Low) @ 19 D-Start: Name @ 19 Exception thrown: 'System.InvalidOperationException' in System.Private.CoreLib.dll

It just blows up on the Name field every time.

Here's the call stack that got me there, but I don't understand who is calling the method!

XBee.Core.dll!XBee.SerialDeviceStream.Read(byte[] buffer, int offset, int count) Line 57 C# BinarySerializer.dll!BinarySerialization.BoundedStream.ReadByteAligned(byte[] buffer, int length) Unknown BinarySerializer.dll!BinarySerialization.BoundedStream.ReadImpl(byte[] buffer, BinarySerialization.FieldLength length) Unknown BinarySerializer.dll!BinarySerialization.BoundedStream.ReadImpl(byte[] buffer, BinarySerialization.FieldLength length) Unknown BinarySerializer.dll!BinarySerialization.BoundedStream.ReadImpl(byte[] buffer, BinarySerialization.FieldLength length) Unknown BinarySerializer.dll!BinarySerialization.BoundedStream.ReadImpl(byte[] buffer, BinarySerialization.FieldLength length) Unknown BinarySerializer.dll!BinarySerialization.BoundedStream.Read(byte[] buffer, int offset, int count) Unknown BinarySerializer.dll!BinarySerialization.TapStream.ReadByteAligned(byte[] buffer, int length) Unknown

@jefffhaynes If you have a minute let me know what you think. I'd really like to use this library!!

andygikling commented 4 years ago

Little more legwork here. I pulled in the source for BinarySerializer.

In BoundedStream.cs, the exception originates from line 555:

protected virtual int ReadByteAligned(byte[] buffer, int length)
{
    return Source.Read(buffer, 0, length);
}

And Source is an XBee.SerialDeviceStream. So there's the issue. Now we just need to understand why SerialDeviceStream doesn't support the non-async read. Or why the BoundedStream makes this call.

I think the call in BoundedStream is needed because it's used quite a bit, but in most cases the Source is a Payload object and supports the non-async Read call...

So why does XBee.SerialDeviceStream not support this non-async Read call?

andygikling commented 4 years ago

Ok so I got to the bottom of this after about 5 hours... In the end, Digi International is dropping the ball. Not surprised.

The 900HP radio firmware version 8075 has apparently changed the format of both of these structures: NetworkDiscoveryResponseData NetworkDiscoveryResponseDataExtendedInfo

In the former, ReceivedSignalStrengthIndicator (DB) is missing. Which causes the Name to not parse correctly!

When the receive message is missing bytes for ReceivedSignalStrengthIndicator, somehow the non-async Read() function does get called. This seems like a bug still either way. But it makes sense why we might not have seen it - when the frame is corrected, and ReceivedSignalStrengthIndicator is removed from NetworkDiscoveryResponseData the BinarySerializer works correctly and I get my data back.

Also note, with my radio's firmware, there are issues with NetworkDiscoveryResponseDataExtendedInfo structure. However, again the issues are not with this library! There are errors in the Digi International documentation! Wtf... The latest online documentation for the SB3 radio has it wrong! Here's a link to their API documentation for the frame in question for my radio:

SB3 900HP Docs

For example, using XCTU a request of (API mode 1): 7E 00 04 08 01 4E 44 64

Responds erroneously with (in my case another node with the name "pva"): 7E 00 1B 88 01 4E 44 00 FF FE 00 13 A2 00 41 63 BF A3 70 76 61 00 FF FE 01 00 C1 05 10 1E F3

Looking closely one will see that we've missed the bytes for signal strength "DB." There is a missing byte between A3 (end of address SL) and 70 (which is "p" - the first char of my NI field).

The we then fall further off the rails - there seems to be no bytes for DEVICE_TYPE or STATUS_RESERVED.

The corrected data structure looks like this:

namespace XBee.Frames.AtCommands
{
    internal class NetworkDiscoveryResponseDataExtendedInfo
    {
        //This works for XBee Pro (SB3) 900HP with Firmware version 8075 in DigiMesh mode.
        //Seems they have changed the response!
        [FieldOrder(0)]
        public ushort ProfileId { get; set; }

        [FieldOrder(1)]
        public ushort ManufactureId { get; set; }

        [FieldOrder(2)]
        public byte[] OptionalData { get; set; }
    }
}

And yes, the last 4 bytes of "OptionalData" in the frame are totally undocumented because my NO value is 0!

Digi... ya' killing me over here.

@jefffhaynes let me know if the following PR is correct or worth while per the bug described above - in my testing it does correctly deal with senario where the response is missing ReceivedSignalStrengthIndicator but you might want to check into why BinarySerializer calls Read() in the first place.

andygikling commented 4 years ago

I closed the above PR because I'm not sure how to fix it exactly. I found more S3B documentation that shows the frame data differently and it seems to align with the response I get more accurately.

Perhaps an older manual, but it starts the ND response with the MY parameter (which is what I've seen in newer manuals). (CORRECTION) I DO see the MY value on my radio and everything else documented here seems accurate. But it's not clear why the MY value is documented and there's still a "ShortAddres" in the data structure - we only get FFFE once...

Sparkfun HP900 datasheet

The above info in this thread is accurate, however I think we need more thought / testing put into the [SerializeWhen] attributes.

I'm going to yield to @jefffhaynes for now and possibly file a support ticket with Digi.

jefffhaynes commented 4 years ago

Sorry, just catching up on this. I’ll try to get into it as soon as possible. I will say the serial device support in UWP is a nightmare, although it sounds like that isn’t the problem.


From: Andy Gikling notifications@github.com Sent: Sunday, November 3, 2019 10:06 AM To: jefffhaynes/XBee Cc: Jeff Haynes; Mention Subject: Re: [jefffhaynes/XBee] throwing InvalidOperationException in Read() (#62)

I closed the above PR because I'm not sure how to fix it exactly. I found more S3B documentation that shows the frame data differently and it seems to align with the response I get more accurately.

Perhaps an older manual, but it starts the ND response with the MY parameter (which is what I've seen in newer manuals). I don't see the MY value on my radio, but everything else after that looks accurate. Sparkfun HP900 datasheethttps://cdn.sparkfun.com/datasheets/Wireless/Zigbee/90002173_N.pdf

The above info in this thread is accurate, however I think we need more thought / testing put into the [SerializeWhen] attributes.

I'm going to yield to @jefffhayneshttps://github.com/jefffhaynes for now and possibly file a support ticket with Digi.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/jefffhaynes/XBee/issues/62?email_source=notifications&email_token=ACKIURYFJ5XZUFMAZ5Y3PYDQR3LIBA5CNFSM4JIBKXXKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC5UYYQ#issuecomment-549145698, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ACKIUR2QL3RVDWNGSWFTC43QR3LIBANCNFSM4JIBKXXA.

FTchips commented 4 years ago

I am encountering this issue as well.

Looking into BinarySerializer, it looks like the AsyncBinaryReader switches from async to sync when it tries to read a char (code below), and this seems to propagate through to the sync read method in SerialDeviceStream.

public Task<char> ReadCharAsync(CancellationToken cancellationToken)
{
    return Task.FromResult(base.ReadChar());
}

( @jefffhaynes Thanks for creating this library. It's Great! )

jefffhaynes commented 4 years ago

Ah, thanks for tracking that down. I'll try to get a fix into the serializer soon.

jefffhaynes commented 4 years ago

Should be fixed in XBee.Core 1.6.6. Please let me know if you have any problems with it.