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

readNonBlocking() and readLatest() don't always call postRead() #116

Closed
8 tasks done
killenb opened this issue Mar 12, 2020 · 5 comments
Closed
8 tasks done

readNonBlocking() and readLatest() don't always call postRead() #116

killenb opened this issue Mar 12, 2020 · 5 comments
Assignees
Labels
readyForImplementation This ticket has been moved to the "ready for implementation" area on the MSK software group board selected This ticket has been selected for the MSK software group board umbrellaChild Child ticked generated from an umbrella ticket

Comments

@killenb
Copy link
Member

killenb commented Mar 12, 2020

Child of ChimeraTK/ApplicationCore#123

readNonBlocking() and readLatest() only call postRead if there was new data. This is wrong. preRead() and postRead() must always be called in pairs, for instance if the user buffer needs to be restored.
In addidion, this will prevent proper exception propagation with the new exception handling scheme.

Solution in order not to perform unnecessary calculations:
change postRead() and doPostRead() to postRead(bool hasNewData) and doPostRead(bool hasNewData), and let the implementations decide what to do in case of no data.

  • Introduce the API change to postRead() and doPostRead() in all places

  • In read() and TransferFuture::wait() and transferGroup::read(), call postRead(true) after a successful read transfer, and postRead(false) if there was an exception

  • In readLatest() and readNonBlocking() call it with the return value of the corresponding doReadXxx if there was no exception, and false if there was an exception

  • Change postWrite to receive a data lost flag (see comment below)

  • Do not change any implementations yet.

  • All tests should still pass. If not, note in this ticket which tests are failing.

  • If there are failing test check if they belong to the follow-up ticket backends must not assign a fresh version number if there was no new data #122. Note in that ticket which corresponding tests are failing.

  • Fix all remaining tests (or the implementations such that the tests pass, depending on which of the two does not correspond to the new spec).

  • Merge into the master

@killenb
Copy link
Member Author

killenb commented Apr 3, 2020

will fix ApplicationCore ExceptionHandling Spec 11.23

@killenb
Copy link
Member Author

killenb commented Apr 6, 2020

The "hasNewData" flag is the return value of "readTransferNonBlocking" or "readTransferLatest". For readTransfer use true.

For symmetry reasons the return value of write shall be added as postWrite(bool dataLost).

@killenb
Copy link
Member Author

killenb commented Apr 6, 2020

Do before of after #119. It introduces an interface change at the same place.

@killenb killenb added umbrellaChild Child ticked generated from an umbrella ticket selected This ticket has been selected for the MSK software group board labels Apr 6, 2020
@killenb killenb added the readyForImplementation This ticket has been moved to the "ready for implementation" area on the MSK software group board label Apr 9, 2020
@killenb killenb added readyForImplementation This ticket has been moved to the "ready for implementation" area on the MSK software group board and removed readyForImplementation This ticket has been moved to the "ready for implementation" area on the MSK software group board labels Apr 14, 2020
@ckampm ckampm self-assigned this Apr 14, 2020
@ckampm
Copy link
Contributor

ckampm commented Apr 14, 2020

The "hasNewData" flag is the return value of "readTransferNonBlocking" or "readTransferLatest". For readTransfer use true.

This statement is contradicting with the second task in the opening comment. After readTransfer, the hasSeenException flag has to be tested in order to check if the transfer has been successful and provided new data.

ckampm pushed a commit that referenced this issue Apr 14, 2020
  - These functions receive bool hasNewData as second argument
  - In readNonBlocking and readLatest, postRead is now called
    unconditionally
ckampm pushed a commit that referenced this issue Apr 14, 2020
ckampm pushed a commit that referenced this issue Apr 14, 2020
These functions now receive the bool dataLost flag
@ckampm
Copy link
Contributor

ckampm commented Apr 14, 2020

,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
readyForImplementation This ticket has been moved to the "ready for implementation" area on the MSK software group board selected This ticket has been selected for the MSK software group board umbrellaChild Child ticked generated from an umbrella ticket
Projects
None yet
Development

No branches or pull requests

2 participants