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

New API #7

Open
wants to merge 58 commits into
base: master
Choose a base branch
from
Open

New API #7

wants to merge 58 commits into from

Conversation

marceloalcocer
Copy link
Member

@marceloalcocer marceloalcocer commented Apr 1, 2021

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 to PandA.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, but PandA.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 tested

3. Changelog

  • Add new interface
  • Add tests
    • New interface
    • Legacy interface
  • Add firmware validation on design load
  • Add port argument to PandA.__init__
  • Add possibility of socket reconnection
  • Add explicit socket close on destruction
  • Ensure full design included in dump
  • Remove save_config.py module
  • Privatize PandA.sock
  • Flatten package hierarchy

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`
@marceloalcocer marceloalcocer mentioned this pull request Apr 1, 2021
@marceloalcocer marceloalcocer changed the title New interface New API Apr 6, 2021
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

Successfully merging this pull request may close these issues.

1 participant