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

Sticky Mac commands management can generate invalid frames #88

Closed
ndejean34 opened this issue May 31, 2016 · 2 comments
Closed

Sticky Mac commands management can generate invalid frames #88

ndejean34 opened this issue May 31, 2016 · 2 comments

Comments

@ndejean34
Copy link

Hello,

While testing the "sticky mac commands" feature, I believe I detected an issue concerning the managemetnt of these commands : the device sometimes generates invalid commands by annoucing a given number of piggy-backed bytes and not copying them into the frame.

I will take as reference the commit "d2c1bbf5d9b7d6b19e1cd90bf6aa01f402bd08f3", labelled "Make MAC commands sticky":

  • lines 2739 to 2741 (comment is // Copy the MAC commands which must be re-send into the MAC command buffer): the sticky commands are copied from MacCommandsBufferToRepeat to the end of MacCommandsBuffer and MacCommandsBufferIndex is updated as it should be
  • lines 2751 to 2754 : only the first bytes of MacCommandsBuffer are copied into the frame and the additional bytes corresponding to the sticky mac commands are ignored

I propose the following 'simple' correction, implying only line 2751:

  • the current code is: for( i = 0; i < ( MacCommandsBufferIndex - MacCommandsBufferToRepeatIndex ); i++ )
  • and "should" become: for( i = 0; i < MacCommandsBufferIndex; i++ )

as MacCommandsBufferIndex is already taking into account the bytes corresponding to the sticky mac commands following its update on line 2741.

Do you agree with my analysis and correction proposal?

Thank you in advance,

@djaeckle
Copy link

Hi ndejean34,

Thanks for your report. I think your analysis is correct. However, I will perform some tests to double check it. I will come back to you when I have more information.

djaeckle pushed a commit that referenced this issue Jun 1, 2016
@djaeckle
Copy link

djaeckle commented Jun 1, 2016

@ndejean34
I was able to reproduce the issue. I have pushed a fix to the develop branch.

@mluis1 mluis1 closed this as completed Jun 3, 2016
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

3 participants