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

Reroute CAN write calls to controlboard device #209

Closed
PeterBowman opened this issue Apr 30, 2019 · 7 comments · Fixed by #229
Closed

Reroute CAN write calls to controlboard device #209

PeterBowman opened this issue Apr 30, 2019 · 7 comments · Fixed by #229

Comments

@PeterBowman
Copy link
Member

PeterBowman commented Apr 30, 2019

In the CanBusControlboard+TechnosoftIpos architecture, write operations are performed in a joint-by-joint manner - that is, whenever we want to IPositionControl::positionMove, the control board forwards demanded positions to one IPositionControlRaw::positionMoveRaw call per joint. Each TechnosoftIpos is therefore responsible for constructing and sending its own message(s) through the CAN bus.

Ideally, we would take advantage of TX/RX yarp::dev::ICanBus buffers (see #159, proposed implementation plan at #208) and send messages in batches. Reception is handled by the read thread in CanBusControlboard. In contrast, transmission is performed in several places, that is, by each TechnosoftIpos device (and others) as explained above. This issue aims to defer the instant in which messages are sent until these are properly formed by each subdevice (TechnosoftIpos). Once collected, let (for instance) CanBusControlboard::positionMove dispatch them in one single call.

Implementation idea:

  • Enhance the ICanBusSharer interface (mind Clean CAN master's DeviceDriver methods #74 (comment)) or create a new one, make TechnosoftIpos and other CAN devices implement it.
  • TechnosoftIpos::positionMoveRaw (and others) would store all messages in a list. This list is managed by the aforementioned interface (perhaps a register(const yarp::dev::CanMessage &) method?).
  • CanBusControlboard::positionMove iterates through all raw subdevices. In this process, per-joint CAN messages are collected, see previous point. This batch of messages is dispatched before leaving the function body.
@PeterBowman
Copy link
Member Author

I'd rather isolate CAN RX/TX so that only the CanBusControlboard can access this device (CanBusHico, CanBusPeak and so on). Therefore, this "master" device would encompass read&write operations as well as routing YARP interfaces (as it does now, but not so effectively as described above).

@PeterBowman
Copy link
Member Author

PeterBowman commented Aug 3, 2019

I'm not sure that TX is attainable in the way I described above, given that certain operations, e.g. mode change, involve a handful of SDO requests that need to be processed sequentially, i.e. sometimes it is required to await a response at step i before proceeding to step i+1. For instance, switching to PT/PVT mode conveys several steps.

@PeterBowman
Copy link
Member Author

RX ready at 8130c70.

@PeterBowman
Copy link
Member Author

PeterBowman commented Aug 5, 2019

Some thoughts on parallelization:

  • Speaking of RX, I don't think it would render any benefit. A single dedicated thread loops over non-blocking reads and redirects incoming CAN messages to the intended receipt via interpretMessage(). In this loop, a crucial delay instruction needed to be hardcoded as a result of Transmit delays in the new ICanBus arquitecture #191. Parallelization could be introduced in a multiple-read scenario (more than one message received in one read call) either by dispatching messages in separate threads that call said interpretMessage(), or via additional read threads that would benefit from the fact that our canRead() implementations are thread safe. Anyway, since interpretMessage() is not supposed to perform heavy operations, the encompassing loop should not be the bottleneck here (rather CAN comms & CAN/system calls, which shall benefit now from message buffering).

  • I would like to introduce threaded writes, though. Assuming that CAN TX comms are entirely managed by CanBusControlboard, this device would start a periodic thread to process incoming write requests. Messages would be registered by raw subdevices into a FIFO queue and dispatched to the network in periodic batches (thus taking advantage of the TX buffer), say every 1-5 milliseconds. The point of this is to parallelize multi-joint queries, so that one or more requests from one joint do not block requests of all other joints (also, this enables message buffering, i.e. we'll send more than one message at once). Incidentally, it would help synchronize (more or less) PDOs without a SYNC signal. Considerations:

    • Even though reads/writes are thread-safe, make sure this new thread does not collide with high-priority RPDOs (e.g. PT/PVT).
    • Raw subdevices will request a free slot from the message buffer, similarly to how prepare() returns YARP bottles in a buffered port context.
    • Most importantly, all of this is meaningless unless multi-joint queries can be actually processed in a parallel manner, e.g. via threaded for-loops. Reminds me of GrPPI; perhaps vanilla C++ (11?) renders sufficient for our needs, though.

@PeterBowman
Copy link
Member Author

PeterBowman commented Aug 12, 2019

TX ready at 3002147. I have refactored read/write threads within CanBusControlboard (for now), also ICanBusSharer has been moved to its own header-only (for now) library, and several dependent apps (programs, examples, tests) have been disabled following #227 (comment).

@PeterBowman
Copy link
Member Author

Most importantly, all of this is meaningless unless multi-joint queries can be actually processed in a parallel manner, e.g. via threaded for-loops. Reminds me of GrPPI; perhaps vanilla C++ (11?) renders sufficient for our needs, though.

Split into #230.

@PeterBowman
Copy link
Member Author

PeterBowman commented Aug 12, 2019

Small follow-up: TX thread cleaned at 20a2390 and CAN devices forced to non-blocking mode. Also, the period is now configurable.

PS interesting read: https://www.oreilly.com/library/view/linux-device-drivers/0596005903/ch06.html

If a process calls write and there is no space in the buffer, the process must block, and it must be on a different wait queue from the one used for reading. When some data has been written to the hardware device, and space becomes free in the output buffer, the process is awakened and the write call succeeds, although the data may be only partially written if there isn't room in the buffer for the count bytes that were requested.

(...) The input buffer is required to avoid losing data that arrives when nobody is reading. In contrast, data can't be lost on write, because if the system call doesn't accept data bytes, they remain in the user-space buffer. Even so, the output buffer is almost always useful for squeezing more performance out of the hardware.

PS2 in case I would ever want to interrupt a blocked device: https://stackoverflow.com/q/6249577. I see little use of blocking RX operations, whereas TX blocks would render harmful given that canWrite and the registering delegate mechanism are locked by the same mutex. That is, to block writes also means to block the prepareMessage callback, which needs to execute fast.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants