mozilla-sensorweb / sensorweb-firmware

Contains all code that runs on the SensorWeb device
Mozilla Public License 2.0
4 stars 3 forks source link

IPC code @dhylands @fdesre #5

Open tdz opened 7 years ago

tdz commented 7 years ago

I think what you really want is a consumer wait queue with more than 1 element. This way a producer can put its message into the queue even if there's contention.

Besides that, I think the code already does want you want. Producers can send as many messages as they want at the same time, and come back later to check for the results:

IPCMessage msg1, msg2; Produce(msg1); IPCQueueConsume(msg1); Produce(msg2); IPCQueueConsume(msg2); // ... do work WaitForReply(msg2); // ... do some more work involving the reply for msg2 WaitForReply(msg1);

For completely async operation, set the NOWAIT flag on the message and there will be no reply from the consumer returned to the producer.

The source of the buffer is up to you. If you want to allocate from a list of buffers, you're free to do so. Originally, I had a struct IPCMessageProducer that maintained the buffer. But I found that it's not required at all. We can add this interface on top of the current patch set, if you prefer buffer lists.

Be aware that shared buffer lists between producers and consumers have contention as well. And this is a form of dynamic allocation with all the problems it has (e.g., contention, starvation, OOMs, leaks).

tdz commented 7 years ago

Oh! Wanting to fix the consumer's queue length, I saw that the queue already has up to 10 elements. [1] That should be more than enough for our use cases.

Since you thought this code is only for synchronous communication, does it help if I add a short tutorial to explain how to use the IPC?

[1] https://github.com/tdz/sensorweb-firmware/blob/ipc/src/IPCQueue.c#L163

tdz commented 7 years ago

I pushed an update to this PR. It fixes two bugs in the VPrint buffer handling and IPC-message cleanup. Most of all it adds 'raw' output functions like PrintFromISR() that work from within ISRs. If you're OK with re-licensing the your string function under MPL 2, I'd like to use them for the implementation of Print, et al. That would allow for removing the VPrintFromISR-internal buffer from the ISR stack.

I also pushed a branch named 'terminal' [1] that implements input over the serial port and a simple command-line interface. That will become my next PR after the basic IPC landed.

[1] https://github.com/tdz/sensorweb-firmware/tree/terminal

dhylands commented 7 years ago

I was probably reading more into things from the comments, rather than the code. I think that an example would probably be good anyways.

tdz commented 7 years ago

I added tutorial-style documentation to the IPC code base and integrated the StrPrint helper functions. One good point about the latter is the reduction of the text-segment size by ~16 KiB.

tdz commented 7 years ago

The content of the PR is completely out of date. I suggest to simply close the PR.