-
Notifications
You must be signed in to change notification settings - Fork 2
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
New API #7
Open
marceloalcocer
wants to merge
58
commits into
master
Choose a base branch
from
new_interface
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
New API #7
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Move socket creation to `connect_to_panda` method so that connections can be opened/closed an arbitrary number of times.
Ensures socket is closed elegantly on PandA destruction.
The blocks referenced in a design must be available in the currently installed FPGA app. This can be ensured in a semi-automatic fashion by specifying the required FW version in the saved design and subsequently validating it when loading a design.
Existing comms methods are quite specific to queries and are thus not very reusable. As such, new more generalized communication methods suitable for queries and assignments were added. The existing comms methods were left to maintain backwards compatibility.
Utilizes new comms methods. Adds handling of all available query responses (error, value, multi-value).
> "Your module package is the core focus of the repository. It should > not be tucked away" > > --- https://docs.python-guide.org/writing/structure/#the-actual-module
Communication with PandABox should be over defined interfaces rather than bare socket. As such, should probably not expose socket instance.
Ensures stable legacy interface during refactoring and new interface development
MockSocket instance passed to tests is reused across tests. As such, must ensure that tests leave mock socket as they found it.
Queries returning zero-length multiple values were raising an exception rather than returning the expected value (empty iterable). Now fixed and test added to assert desired behaviour.
Unexpected exception found when assigning empty tables. Adding empty assignment (single value and table) tests to develop fix.
New design dump/load methods provide improved functionality. As such, refactor legacy interfaces to use these. Legacy interfaces ensured stable after refactoring by legacy tests.
Socket interface behaviour depends on both local and remote socket connection states, e.g. socket.recv will raise an exception when disconnected locally, but will return zero bytes when disconnected remotely. Added _connected_remote attribute to provide more realistic mocking of remote disconnections.
Mock socket timeouts using network attribute. N.b. This assumes the mock socket is operating in timeout mode
Probably best to describe how to run tests with manual path modification in README, i.e. `PYTHONPATH=pandaboxlib python3 tests/test_panda.py`
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
1 . Concept
The previous public API and implementation was somewhat brittle with repeated code, loose response validation, little testing, etc. This PR aims to make the library more robust by introducing a new streamlined API for socket-based communication with PandABox units.
The legacy API has been retained however to ensure backwards compatibility for the near future. Compatibility has been ensured by adding legacy API tests prior to new API development.
This PR supersedes all the proposed changes introduced in #5.
2. Review notes
2.1. New interface
The new interface centres around three main communication methods;
PandA.query_
PandA.assign
PandA.assign_table
N.b. the trailing underscore in
PandA.query_
to ensure the legacy method of the same name is not shadowed. It is envisaged that this can be renamed toPandA.query
once the legacy interface is retired.2.2. Legacy interface refactoring
Some, but not all the methods in the legacy API have been refactored to use the new communication methods, e.g.
PandA.save_config
has been migrated, butPandA.send_seq_table
has not. It is envisaged that this migration should be completed soon.2.3. Distribution
Distribution (i.e. execution of
setup.py
) has not yet been tested3. Changelog
PandA.__init__
save_config.py
modulePandA.sock