Add a circular buffer cirbuf module

[hvegh]

The cirbuf module provides a simple, in memory, circular buffer. This simplifies the design of typical workloads like the sampling of sensors at regular intervals. The memory usage is efficient, apart from ~20 bytes for administration of the circular buffer no additional overhead is required to store the data.

[hvegh]

I had some troubles with git, so I deleted my hve/nodemcu tree and reforked in order to get a single commit. so this is related to pull request #1060 sorry for that.

[hvegh]

Also contemplating to introduce read handles... which permit multiple independent readers on the same buffer. For example when we have multithreading in nodemcu.

h = cirbuf.new(num,len)

rh, buf = cirbuf.readstart(h  [, maxnum])
buf = cirbuf.readnext(rh, [,maxnum])

state of the reader is maintained in the rh handle...

or we could use this:
rh = cirbuf.open(h)
buf = cirbuf.read(rh, [,maxnum])

Maybe this is even better...

[hvegh]

Just for the fun of it: here I use an am2320 sensor module and store a reading every 30 seconds in cirbuf. Nodemcu is a standalone wifi-station with webserver for the temp/humidity chart.

[pjsg]

I think this should be next position (and the syntax suffers from block-copy-itis as well)

[hvegh]

Yes you're right I will fix it.

[pjsg]

I'm not very happy about the API -- it seems that there is a confusion between strings and numbers, and, in order to use it, you need to understand how it works.

You can push in one or more elements at a time. This is fine, but they don't come out like that. They come out in a blob (and it isn't clear that you are guaranteed to get a whole number of elements out).

What happens with float values?

It seems that this code is combining the circular buffer aspect with the structure packing aspect, and that seems wrong to me. There is the struct module for doing packing already...

[TerryE]

In my view the idea of a circular buffer is a good one, but I don't like this implementation. Better in my view just to have string argument (not a fixed length) with puts() and putf() methods, with the second essentially a wrapper for string.format() and also a save() method to write the buffer to SPIFFS.

[hvegh]

@pjsg
Floats unfortunately are not covered.
I did not consider struct because it only seems to have byte granularity. And I wanted every byte to count since it is on the heap. Instead of 4 bytes i use 3 bytes (10 bits humidity 14 bits temp) 17280 bytes used for 2 days of 30 sec samples...

The backend converts numbers to 4 bytes in LE format. I wanted to be able to combine the use of strings and numbers. As numbers seemed to be more easily manipulated in Lua:

function sample()
  local h,t
  if pcall(function()
    h, t =am2320.read()
    end) then
    t=t*1024
    cirbuf.push(b, h+t)
  else
    cirbuf.push(b, 0)
  end
end

I do agree that the current cirbuf has to many shortcuts in order to fit my use case ....

[TerryE]

@hvegh, your case is probably at the extreme end of the possible applications. If we limit the Lua string size to 256 then this is only a 1-byte overhead per string or we can go for a simple high bit multi-byte encoding allowing longer strings with an overhead of 2 bytes for strings over 128 bytes, and the possible usecases would be far broader.

We really need a broader application for it to be a standard library, IMO.

[hvegh]

@TerryE

.. but I don't like this implementation.

Fair enough, it was purely intended as a sensor buffer.
Requirements I had in mind:

• I don't want to have to worry about wearing out flash. So use heap.
• Since we use heap, make it as memory efficient as possible:
  • Sensor data is fixed size. so cirbuf objects are fixed size this saves worrying about variable sizes. and keeps the code simple.
• keep it simple :)

Better in my view just to have string argument (not a fixed length) with puts() and putf() methods,

The circular buffer objects are fixed size, variable size buffer objects would add a level of complexity which the current code does not cover.
For the push I just wanted to allow for easily concatenating strings and numbers as a convenience. All arguments in a push are concatenated but will be never exceed the cirbuf object size. Floats are not covered mainly because they are hard to do well when you want to store your data in an interchangeable format.

with the second essentially a wrapper for string.format() and also a save() method to write the buffer to SPIFFS.

Wouldn't it be more convenient to handle the writing to SPIFFS in lua (Otherwise you might risk including SPIFFS specifics in your buffer module...)??

I would be totally willing to put some effort in changing the module, but for me it is unclear what for you guys a minimum acceptable product is. What would be the first acceptable increment of the code?

[marcelstoer]

@TerryE & @pjsg do you intend to continue help shape this module or would you rather see this PR closed?

[pjsg]

After this length of time, I think that we should close this.

[TerryE]

@marcelstoer, I've got my own history library which is more flexible for my needs, and I am absolutely strapped for free time, so I don't want to spend any time on this. So this is really one for Phil to decide --

PS. Sorry missed Phil's comment so +1 to closing.

[marcelstoer]

If I look at your previous comments (Philips' and Terry's) I conclude that you both think it'd be worthwhile to have this module. However, you weren't happy with the initial implementation. Henk offered to update the code so that you'd accept it. I'd feel bad for Henk to just close it like that.
As I'm not the one who could give him feedback as for how the change the code my vote doesn't really count, though.

[TerryE]

@marcelstoer, OK, but Henk rejected my comments, so I just decided that it was easier to write my own library rather than have an argument, which I did. Now I've done that and got it working fine.

I don't mind keeping this open, but my priority on this is still low, now that I have my own version. Sorry.

[hvegh]

Regarding: #1062 (comment) @terry, Don't want to spoil the party, looking forward to your "history" module. Cheers

[marcelstoer]

Terry, I only just realized that your PR #1199 is a substitute for this one, sorry.