Consider changing 'pb' to an instance

[surfskidude]

Thank you for providing this Lua module. I have tested your implementation with MQTT Sparkplug
https://www.eclipse.org/tahu/spec/Sparkplug%20Topic%20Namespace%20and%20State%20ManagementV2.2-with%20appendix%20B%20format%20-%20Eclipse.pdf

Your code can compile, encode, and decode Sparkplug B Google Protocol Buffer Schema and messages.

Having said that, I noticed something unusual in your implementation. The pb module is not designed as a Lua userdata object and modifications to pb is directly in the global environment. That means in a large application, everyone work on the same instance, which can make it difficult to use. For example, one module may call pb.clear() and invalidate for all users of the module.

I propose that the code instead is change to work as follows:

From:

local pb = require "pb"
pb.load(data)

To:

local pb = require "pb".new()
pb:load(data)

[starwing]

You could use multiple state to implement this:

local oldstate = pb.state(nil) -- fetch the old, and make a new instead
local newstate = pb.state() -- now you can fetch the new state
-- use it:
protoc.reload()
-- restore the old state
pb.state(oldstate)
-- or use the new state instead
pb.state(newstate)

The reason to make API this way is for compatible with earlier intreface :-(

[surfskidude]

Thank you for your suggestion, but I am concerned that the extra overhead with swapping states may make the code too slow. I may have to process Sparkplug MQTT messages at a high rate.

Please consider the following backward compatible solution for future releases:

Move the existing C code into a new module named "pbuf".
Modify this C code so it can create new instances and work with instances (userdata).
Create a new module named "pb" in Lua. This Lua module, which makes the new version backward compatible, would look like:
local pb=require"pbuf".new()
return {
load=function(...) return pb:load(...) end,
encode=function(...) return pb:encode(...) end,
etc....
}

[starwing]

Maybe you could try it out to see how swap states slow down the process.

This module is designed to just compiled as a single module for "batteries contained", so with a second needed Lua script is not acceptable by design.

Maybe i could add some methods to the state object for support methods accessibility, but it should be proved that's necessary.

[starwing]

I have tried to make state object has methods this weekend but found it's difficult. because not only load/encode/decode need state, all functions except "tohex/fromhex/result" all need state as well as some functions from "pb.conv" module. To change current API to state methods needs huge change for code structure. Do you have some idea about this?

[surfskidude]

Well, I looked at your code and it would take me considerable time to understand the details, but I have some suggestions, which I have briefly explained before. The C module would not be backward compatible, but you can easily make it backward compatible by creating a Lua wrapper (module) as I previously explained. You already have an object, but you store it in the registry. In fact, the current C code is in my humble opinion more complicated than it would have been if you simply had worked on an instance.

I would move all state management functions such as pb.state() to the backward compatible module, which will now be in Lua. These function will not be needed in the new C module.

function lpb_lstat() should be redesigned to return the userdata object e.g.

LUALIB_API lpb_State *lpb_lstate(lua_State *L) {
return (lpb_State *)luaL_checkudata(L,1,PB_STATE) ;
}

However, all functions taking arguments must now be changed as the PB_STATE is now on position 1 -- e.g.

static int Lpb_encode(lua_State *L) {
lpb_State *LS = lpb_lstate(L);
const pb_Type *t = lpb_type(LS, lpb_checkslice(L, 2)); // from 1 to 2

[starwing]

That means all functions using state must be the methods of state. But those functions are not only in pb module, but also in other modules.