Skip to content

Realtek-rtl8195am-Network_Socket_Updates #9084

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

Merged

Conversation

AdamZhang0124
Copy link
Contributor

@AdamZhang0124 AdamZhang0124 commented Dec 13, 2018

This PR addresses the issue of #8124.
It updates and enriches the wifi connection error type to adapt the Network Socket test plan requirement.
In the meantime, it increases the heap size that allows the transmission of larger packet size.

Description

  1. Increase heap size in lwipstack\mbed_lib.json to fulfill bursty TCP and UDP transmission requirement.
  2. Modify and enrich wifi connection error types in TARGET_AMEBA\RTWInterface.cpp to adapt the decision logic of the wifi test cases.
  3. Add new static constants in TARGET_AMEBA\RTWInterface.h, including 'SSID_MAX_LENGTH', 'PASSPHRASE_MAX_LENGTH' and 'PASSPHRASE_MIN_LENGTH' to help verifying the validity of ssid and passphrase.

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

@AdamZhang0124
Copy link
Contributor Author

AdamZhang0124 commented Dec 13, 2018

Hi @0xc0170 and @cmonr ,
Thank you for the comments on the pull request #9070 .
I have amended the pull request and removed the redundant commits. Please help review it again. Sorry for the inconvenience caused.

@ciarmcom
Copy link
Member

@AdamZhang0124, thank you for your changes.
@ARMmbed/mbed-os-ipcore @ARMmbed/mbed-os-maintainers please review.

@0xc0170
Copy link
Contributor

0xc0170 commented Dec 13, 2018

Thanks for the update, looks better. Realtek-rtl8195am-Network Socket Updates - can you amend to your commits what updates are being done

from the previous PR: Increase heap size in lwipstack\mbed_lib.json. Modify and enrich wifi connection error types in TARGET_AMEBA\RTWInterface.cpp Add new parameters in TARGET_AMEBA\RTWInterface.h. This would be helpful to have in the commit message. Why we are increasing heap size? Why are new parameters needed in RTWInterface, and similar questions answered there.

Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Describe changes in the commit message (provide details)

@AdamZhang0124
Copy link
Contributor Author

Thanks for the update, looks better. Realtek-rtl8195am-Network Socket Updates - can you amend to your commits what updates are being done

from the previous PR: Increase heap size in lwipstack\mbed_lib.json. Modify and enrich wifi connection error types in TARGET_AMEBA\RTWInterface.cpp Add new parameters in TARGET_AMEBA\RTWInterface.h. This would be helpful to have in the commit message. Why we are increasing heap size? Why are new parameters needed in RTWInterface, and similar questions answered there.

Hi @0xc0170, thanks for the feedback. I have amended the commit message. Please let me know if there is anything need to be further explained. Thanks.

@0xc0170
Copy link
Contributor

0xc0170 commented Dec 14, 2018

@AdamZhang0124 not yet pushed? I dont see any change here as for now

@AdamZhang0124
Copy link
Contributor Author

@AdamZhang0124 not yet pushed? I dont see any change here as for now

Up on request, I have commented the reasons of the file changes under Description section already.
In addition, this pull request mainly aims to fix the the issue of #8124 . More detailed information and change history can be found at the comment area of #8124 . Thanks.

@0xc0170
Copy link
Contributor

0xc0170 commented Dec 18, 2018

That is correct, it is good to have it described here.
However for people working offline, or basically outside of Github, there is no reference to the issue, this pull request only very short sentence that does not say much. It's worth describing it in the commit msg (I often copy commit message to the Pull request description). Our docs reference this article as a guide for commit messages: https://chris.beams.io/posts/git-commit/#seven-rules

If you can quickly amend your commit msg, we shall start Ci right away

@AdamZhang0124
Copy link
Contributor Author

AdamZhang0124 commented Dec 18, 2018

That is correct, it is good to have it described here.
However for people working offline, or basically outside of Github, there is no reference to the issue, this pull request only very short sentence that does not say much. It's worth describing it in the commit msg (I often copy commit message to the Pull request description). Our docs reference this article as a guide for commit messages: https://chris.beams.io/posts/git-commit/#seven-rules

If you can quickly amend your commit msg, we shall start Ci right away

@0xc0170 Oh, I see what you mean. I thought you mean the description on this page is not clear.
After checking, I do have missed out the commit message. Sorry about that.
Now I am looking for a way to add it. It seems like I cannot edit commit msg directly at here. So, instead of submit a new pull request, do you have more convenient and straight forward ways to do it?
Thanks.

@0xc0170
Copy link
Contributor

0xc0170 commented Dec 18, 2018

The last commit can be amended git commit --amend or git rebase -i HEAD~1 in this case. Then you need to force push to your branch.

This PR addresses the issue of ARMmbed#8124.
It updates and enriches the wifi connection error type to adapt the Network Socket test plan requirement.
In the meantime, it increases the heap size that allows the transmission of larger packet size.

Description
1. Increase heap size in lwipstack\mbed_lib.json to fulfill bursty TCP and UDP transmission requirement.
2. Modify and enrich wifi connection error types in TARGET_AMEBA\RTWInterface.cpp to adapt the decision logic of the wifi test cases.
3. Add new static constants in TARGET_AMEBA\RTWInterface.h, including 'SSID_MAX_LENGTH', 'PASSPHRASE_MAX_LENGTH' and 'PASSPHRASE_MIN_LENGTH' to help verifying the validity of ssid and passphrase.

Pull request type
[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change
@AdamZhang0124 AdamZhang0124 force-pushed the realtek-rtl8195am-Network_Socket_Update branch from c417248 to 1422379 Compare December 18, 2018 09:31
@AdamZhang0124
Copy link
Contributor Author

AdamZhang0124 commented Dec 18, 2018

The last commit can be amended git commit --amend or git rebase -i HEAD~1 in this case. Then you need to force push to your branch.

@0xc0170
Thanks a lot for the help. I have amended the commit message. Please kindly help to check if it is ok. Thanks.

@0xc0170
Copy link
Contributor

0xc0170 commented Dec 18, 2018

CI started

@mbed-ci
Copy link

mbed-ci commented Dec 18, 2018

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

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

Successfully merging this pull request may close these issues.

6 participants