Skip to content

Conversation

mtomczykmobica
Copy link

Description

In mbed-os/TESTS/netsocket/README.md we have example of setup test server only for unsecured sockets. This changes added manual how to setup test server for tls socket version.

Pull request type

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

Reviewers

@SeppoTakalo

Release Notes

@@ -0,0 +1,31 @@
#include "tls_tests.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

add license header please

Copy link
Author

Choose a reason for hiding this comment

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

Done.

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 10, 2019

@AnotherButler Please review

Edit document.
Copy link
Contributor

@AnotherButler AnotherButler left a comment

Choose a reason for hiding this comment

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

Please address my comments.

1. Call `Socket::close(stack)`.
1. Call `Socket::open(stack)`.
1. Call `Socket::close(stack)`.
1. Delete the socket.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a difference between "Delete the socket" and "Destroy the socket"? We use "destroy" everywhere else.

Copy link
Author

Choose a reason for hiding this comment

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

I think both words are correct (means the same), but for consistency we can use "destroy" everywhere.

1. Call `TCPSocket::recv(<buffer>, 30);`.
1. Repeat until `recv()` returns 0.
1. Call `TCPSocket::close();`.
1. Delete the socket.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a difference between "Delete the socket" and "Destroy the socket"? We use "destroy" everywhere else.

Copy link
Author

Choose a reason for hiding this comment

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

I think both words are correct (means the same), but for consistency we can use "destroy" everywhere.


Second recv() should return zero because endpoint closed the connection.
close() should return `NSAPI_ERROR_OK'.
The first `recv()` returns more that zero. Something between 10 and 30 bytes (datetime string).
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this saying? That is returns more than 0? That it returns between 10 and 30? Both?

Copy link
Author

Choose a reason for hiding this comment

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

Changed this sequence to:
The first recv() returns datetime string length (It is between 10 and 30 bytes)

### TLSSOCKET_SEND_REPEAT

**Description:** Run SOCKET_SEND_REPEAT for TLSSOCKET by using port number 2009.
**Description:** Run `SOCKET_SEND_REPEATÂ` for TLSSOCKET by using port number 2009.
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like there's some weird formatting going on. Do we know what this symbol should be, or should we delete it?

### TLSSOCKET_SEND_TIMEOUT

**Description:** Run SOCKET_SEND_TIMEOUT for TLSSOCKET by using port number 2009.
**Description:** Run `SOCKET_SEND_TIMEOUTÂ` for TLSSOCKET by using port number 2009.
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like there's some weird formatting going on. Do we know what this symbol should be, or should we delete it?

Copy link
Author

@mtomczykmobica mtomczykmobica Sep 11, 2019

Choose a reason for hiding this comment

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

Sorry but i don't know that. I propose to leave your changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

should be SOCKET_SEND_TIMEOUT, lets remove the last char

Copy link
Author

@mtomczykmobica mtomczykmobica Sep 11, 2019

Choose a reason for hiding this comment

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

Yes, you're right. I've removed it.

### TLSSOCKET_ECHOTEST

**Description:** Run SOCKET_ECHOTEST for TLSSOCKET by using port number 2007.
**Description:** Run `SOCKET_ECHOTESTÂ` for TLSSOCKET by using port number 2007.
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like there's some weird formatting going on. Do we know what this symbol should be, or should we delete it?

### TLSSOCKET_ECHOTEST_NONBLOCK

**Description:** Run SOCKET_ECHOTEST_NONBLOCK for TLSSOCKET by using port number 2007.
**Description:** Run `SOCKET_ECHOTEST_NONBLOCKÂ` for TLSSOCKET by using port number 2007.
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like there's some weird formatting going on. Do we know what this symbol should be, or should we delete it?

Copy link
Author

@mtomczykmobica mtomczykmobica Sep 11, 2019

Choose a reason for hiding this comment

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

Sorry but i don't know that. I propose to leave your changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to above, the last char should be deleted

Copy link
Author

@mtomczykmobica mtomczykmobica Sep 11, 2019

Choose a reason for hiding this comment

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

Yes, you're right. I've removed it.

**Description:**

Run TCPSOCKET_RECV_TIMEOUT for TLSSOCKET by using port number 2007.
Run `TCPSOCKET_RECV_TIMEOUTÂ` for TLSSOCKET by using port number 2007.
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like there's some weird formatting going on. Do we know what this symbol should be, or should we delete it?

Copy link
Author

@mtomczykmobica mtomczykmobica left a comment

Choose a reason for hiding this comment

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

All changes after review added.

`Socket::open()` should first return `NSAPI_ERROR_OK` and second call `NSAPI_ERROR_PARAMETER`.


`Socket::open()` first returns `NSAPI_ERROR_OK` and then calls `NSAPI_ERROR_PARAMETER`.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is now incorrect. Both NSAPI_ERROR_OK and NSAPI_ERROR_PARAMETER are return values that Socket::open() can return.

When Socket::open() is first called, it returns NSAPI_ERROR_OK, when Socket::open() is called second time, it returns NSAPI_ERROR_PARAMETER.

Copy link
Author

@mtomczykmobica mtomczykmobica Sep 12, 2019

Choose a reason for hiding this comment

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

Fixed that by change to more detailed description:
first call (socket was not opened) returns NSAPI_ERROR_OK and nexts calls (socet was opened by previous call Socket::open()) return NSAPI_ERROR_PARAMETER.

**Description:**

Call `Socket::open()` followed by `Socket::close()` and then again `Socket::open()`. Should allow you to reuse the same object.
Call `Socket::open()` followed by `Socket::close()` and then again `Socket::open()`. This allows you to reuse the same object.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should stay in the form Should allow as we are testing at it is allowed.

Stack SHOULD allow us to do it, but it may fail to do so.

We are not the ones who have enabled, therefore this allows is a bit incorrect statement.

Copy link
Author

Choose a reason for hiding this comment

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

Back to previous version.

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 18, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Sep 18, 2019

Test run: SUCCESS

Summary: 4 of 4 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.

5 participants