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.

Sign up for GitHub

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

Jump to bottom

feat: ComAux ability to clear receiving buffer #86

Merged
merged 1 commit into from
Aug 1, 2022
Merged

feat: ComAux ability to clear receiving buffer #86

merged 1 commit into from
Aug 1, 2022

Conversation

Mcthomas777
Copy link
Contributor

No description provided.

@Mcthomas777 Mcthomas777 changed the title [OT TO REVIEW] feat: ComAux ability to clear receiving buffer [NOT TO REVIEW] feat: ComAux ability to clear receiving buffer Jul 6, 2022
@Mcthomas777 Mcthomas777 changed the title [NOT TO REVIEW] feat: ComAux ability to clear receiving buffer feat: ComAux ability to clear receiving buffer Jul 6, 2022
sebastianpfischer
sebastianpfischer requested changes Jul 6, 2022
View reviewed changes
Copy link
Contributor

@sebastianpfischer sebastianpfischer left a comment

Choose a reason for hiding this comment

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

Documentation and additional parts also missing
What's new?
Robot framework not updated
Examples missing?

src/pykiso/lib/auxiliaries/communication_auxiliary.py Outdated Show resolved Hide resolved
src/pykiso/lib/auxiliaries/communication_auxiliary.py Outdated Show resolved Hide resolved
src/pykiso/lib/auxiliaries/communication_auxiliary.py Outdated Show resolved Hide resolved
tests/test_com_aux.py Outdated Show resolved Hide resolved
@Mcthomas777 Mcthomas777 force-pushed the Extend-ComAux-with-a-clear-buffer-functionality branch from 126fcd8 to 721669e Compare July 6, 2022 09:00
BKaDamien
BKaDamien suggested changes Jul 6, 2022
View reviewed changes
src/pykiso/lib/auxiliaries/communication_auxiliary.py Outdated Show resolved Hide resolved
src/pykiso/lib/auxiliaries/communication_auxiliary.py Outdated Show resolved Hide resolved
src/pykiso/lib/auxiliaries/communication_auxiliary.py Show resolved Hide resolved
src/pykiso/lib/auxiliaries/communication_auxiliary.py Outdated Show resolved Hide resolved
tests/test_com_aux.py Outdated Show resolved Hide resolved
tests/test_com_aux.py Outdated Show resolved Hide resolved
tests/test_com_aux.py Outdated Show resolved Hide resolved
@Mcthomas777 Mcthomas777 force-pushed the Extend-ComAux-with-a-clear-buffer-functionality branch 2 times, most recently from 192ed8c to e07b80d Compare July 6, 2022 11:37
@codecov
Copy link

codecov bot commented Jul 6, 2022 •
edited
Loading

Codecov Report

Merging #86 (b799925) into master (b799925) will not change coverage.
The diff coverage is n/a.

❗ Current head b799925 differs from pull request most recent head 4f70482. Consider uploading reports for the commit 4f70482 to get more accurate results

@@           Coverage Diff           @@
##           master      #86   +/-   ##
=======================================
  Coverage   94.72%   94.72%           
=======================================
  Files          68       68           
  Lines        4906     4906           
=======================================
  Hits         4647     4647           
  Misses        259      259

Help us with your feedback. Take ten seconds to tell us how you rate us.

@Mcthomas777 Mcthomas777 force-pushed the Extend-ComAux-with-a-clear-buffer-functionality branch from e07b80d to 16ad81d Compare July 6, 2022 12:23
@sebastianpfischer
Copy link
Contributor

So after our discussion:

  • When read request API is called, we provide to the receiver a flag that will let one message pass
  • We define a context manager & decorator to allow the gathering of many messages

@Mcthomas777 Mcthomas777 force-pushed the Extend-ComAux-with-a-clear-buffer-functionality branch 2 times, most recently from 04b9f67 to f5f53ab Compare July 11, 2022 14:30
@Mcthomas777 Mcthomas777 force-pushed the Extend-ComAux-with-a-clear-buffer-functionality branch 2 times, most recently from 2b8f051 to 0692b2a Compare July 11, 2022 20:07
@Mcthomas777 Mcthomas777 force-pushed the Extend-ComAux-with-a-clear-buffer-functionality branch from 0692b2a to 4416781 Compare July 12, 2022 11:45
sebastianpfischer
sebastianpfischer requested changes Jul 12, 2022
View reviewed changes
docs/whats_new/version_ongoing.rst Outdated Show resolved Hide resolved
docs/whats_new/version_ongoing.rst Outdated Show resolved Hide resolved
@Mcthomas777 Mcthomas777 force-pushed the Extend-ComAux-with-a-clear-buffer-functionality branch from 4416781 to 2e08240 Compare July 13, 2022 06:17
Pog3k
Pog3k requested changes Jul 14, 2022
View reviewed changes
Copy link
Contributor

@Pog3k Pog3k left a comment

Choose a reason for hiding this comment

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

Check Robot Version.

Or Create ticket to fix in future

@Mcthomas777 Mcthomas777 force-pushed the Extend-ComAux-with-a-clear-buffer-functionality branch 2 times, most recently from 570cabf to a16503f Compare July 14, 2022 11:29
sebastianpfischer
sebastianpfischer requested changes Jul 18, 2022
View reviewed changes
Copy link
Contributor

@sebastianpfischer sebastianpfischer left a comment

Choose a reason for hiding this comment

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

There are some additional changes to do (like discussed offline)

src/pykiso/lib/robot_framework/communication_auxiliary.py Show resolved Hide resolved
src/pykiso/lib/auxiliaries/communication_auxiliary.py Outdated Show resolved Hide resolved
src/pykiso/lib/auxiliaries/communication_auxiliary.py Show resolved Hide resolved
@Mcthomas777 Mcthomas777 force-pushed the Extend-ComAux-with-a-clear-buffer-functionality branch 2 times, most recently from 7816eb7 to 67d05c6 Compare July 18, 2022 13:08
@Mcthomas777 Mcthomas777 force-pushed the Extend-ComAux-with-a-clear-buffer-functionality branch 2 times, most recently from 0eff4d1 to fbd7bef Compare July 18, 2022 15:03
sebclrsn
sebclrsn reviewed Jul 19, 2022
View reviewed changes
examples/templates/suite_com/test_com.py Show resolved Hide resolved
docs/whats_new/version_ongoing.rst Outdated Show resolved Hide resolved
docs/whats_new/version_ongoing.rst Outdated Show resolved Hide resolved
sebastianpfischer
sebastianpfischer requested changes Jul 19, 2022
View reviewed changes
Copy link
Contributor

@sebastianpfischer sebastianpfischer left a comment

Choose a reason for hiding this comment

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

Based on our discussion, the code does not provide the required features yet

@Mcthomas777 Mcthomas777 force-pushed the Extend-ComAux-with-a-clear-buffer-functionality branch from fbd7bef to 56b6d57 Compare July 19, 2022 12:55
@Mcthomas777 Mcthomas777 force-pushed the Extend-ComAux-with-a-clear-buffer-functionality branch 2 times, most recently from 73d2b35 to e82d5b8 Compare July 19, 2022 13:09
sebastianpfischer
sebastianpfischer requested changes Jul 19, 2022
View reviewed changes
examples/templates/suite_com/test_com.py Outdated
self.assertEqual(response, b"\x02\x04\x06")
# check messages have not been queued in as
# context manager wasn't used
self.assertTrue(com_aux.queue_out.empty())
Copy link
Contributor

Choose a reason for hiding this comment

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

From the test above, there is no element known that will populate the queue_out. Am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To populate the queue send_message need to be in the ctx manager, otherwise you can not retrieve any messages

src/pykiso/lib/auxiliaries/communication_auxiliary.py Outdated Show resolved Hide resolved
src/pykiso/lib/auxiliaries/communication_auxiliary.py Outdated Show resolved Hide resolved
src/pykiso/lib/robot_framework/communication_auxiliary.py Outdated Show resolved Hide resolved
src/pykiso/lib/robot_framework/communication_auxiliary.py Outdated Show resolved Hide resolved
@Mcthomas777 Mcthomas777 force-pushed the Extend-ComAux-with-a-clear-buffer-functionality branch 2 times, most recently from 11ee349 to 0ace812 Compare July 20, 2022 08:58
@Mcthomas777 Mcthomas777 force-pushed the Extend-ComAux-with-a-clear-buffer-functionality branch from 0ace812 to 4f70482 Compare August 1, 2022 09:45
@sebclrsn sebclrsn merged commit e69cebc into eclipse:master Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sebclrsn sebclrsn sebclrsn approved these changes

@TedRio TedRio TedRio left review comments

@Pog3k Pog3k Pog3k approved these changes

@BKaDamien BKaDamien BKaDamien approved these changes

@sebastianpfischer sebastianpfischer Awaiting requested review from sebastianpfischer

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Extend communication-aux with a buffer clear functionality
6 participants
@Mcthomas777 @sebastianpfischer @Pog3k @sebclrsn @BKaDamien @TedRio