Closed disRecord closed 6 years ago
I added getOperationInfo() to TaskContext. getOpInfo() is preserved as alias.
Also note that TaskContext interface still contains getOps() instead of getOperationNames()
Are there any other comments? This functionality allows to implement lua completion for operations in services: https://github.com/orocos-toolchain/rttlua_completion/pull/6 .
Looks good to me.
Few side comments:
luaL_error
triggers, destructors for local variables at line #1766 are not called. Thus args
variable construction is better to be performed after checks that could lead to luaL_error
call.lua_pushstring(L, s.c_str())
could be replaced with lua_pushlstring(L, s.c_str(), s.size())
with explicit size specification, because std::string
already caches size of its storage, thus one strlen
call, performed by lua_pushstring
internally, may be eliminated.lua_newtable(L);
at line #1777 may be replaced with lua_createtable(L, args.size(), 0);
(args.size()
array elements).it++
with ++it
at line #1779lua_newtable(L);
at line #1780 may be replaced with lua_createtable(L, 0, 3);
(3 record elements).lua_pushstring(L, "name"); lua_pushstring(L, it->name.c_str()); lua_rawset(L, -3);
may be replaced with lua_pushlstring(L, it->name.c_str(), it->name.size()); lua_setfield (L, -2, "name");
Well... These comments concerns not only provided code fragment but the entire file. If we want to preserve the same code style in entire file a major refactoring is needed. I do not think it should be performed in this pull request.
@disRecord I advise to consider these notes at least in the code chunk you suggested to add. It's up to you and organization members to decide whether introduce them in other places.
@meyerj Are you going to merge this PR?
These changes fixes the following problem. If in lua console user tries to display Service interface and one of Operations contains
unknown_t
as parameter then lua exception occurs.TaskContext
interfaces are displayed properly so I modifiedservice2str
accordingly.