> This is moving slowly. I now have RPC caller side support for std::string and 
> std::vector<char>. RPC server side is next. K.O.

The RPC_CXX code is now merged into MIDAS branch feature/rpc_call_cxx.

This code fully supports passing std::string and std::vector<char> through the MIDAS RPC is both directions.

For data passed from client to mserver, memory for string and vector data is allocated automatically as needed.

For data returned from mserver to client, memory to hold returned string and vector data is allocated automatically as 
need.

This means that RPC calls can return data of arbitrary size, the rpc caller does not need to know maximum data size.

Removing this limitation was the main motivation for this development.

I completed this code in June 2024, but could not merge it because I broke my git repository (oops). Now I am doing 
the merge manually. Changes are isolated to rpc_call_encode(), rpc_call_decode() and rpc_execute(). My intent right 
now is to use the new RPC code only for RPCs that pass std::string and std::vector<char>, existing RPCs will use the 
old code without any changes. This seems to be the safest way to move forward.

Included is test_rpc() which tests and probes most normal uses cases and some corner cases. When writing the test 
code, I found a few bugs in the old MIDAS RPC code. If I remember right, I committed fixes for those bugs to main 
MIDAS right then and there.

K.O.

I updated mvodb and test_mvodb. MIDAS ODB and JSON ODB now implement all API 
functions. ReadKey, ReadDir and ReadKeyLastWritten were previously missing from 
some implementations.

I do not remember any other bugs or problems in mvodb, if you want me to add, fix 
or change something, please speak up!

K.O.

Dear MIDAS experts,

there is a [small] number of places in the ODB tree, like "Logger/Data dir" or "Custom/Path" which contain paths,
and where one could benefit from using environment variables. 

For example, instead of setting "Custom/Path" explicitly to "/data/custom/path", one could have is set to 
"${CUSTOM_PATH}", where $CUSTOM_PATH would be defined in the environment. This 

Alternatively, could argue that using env vars would make ODB less explicit and have other side effects

Just in time, enter std::string_view.
https://stackoverflow.com/questions/40127965/how-exactly-is-stdstring-view-faster-than-const-stdstring

I was looking at https://root.cern/doc/v638/classROOT_1_1Experimental_1_1RFile.html and they use it everywhere instead of 
std::string and const char*.

(so now we have 4 string types to deal with, counting ROOT's TString).

P.S. For extra safety, this code compiles, then explodes:

std::string_view get_temporary_string() {
  std::string s = "temporary";
  return s; // DANGER! 's' is destroyed, view dangles.
}

K.O.

> > 2) "advanced" c++ code:
> > 
> > void foo(const std::string& xxx) { ... };
> > int main() { foo("bar"); }
> > 
> > copy-created 2nd string is avoided, but string object to hold "bar" is still must be 
> > made, 1 malloc(), 1 memcpy().
> 
> Are you sure about this? I always thought that foo only receives a pointer to xxx which it puts on the stack, so
> no additional malloc/free is involved.

Yes, "bar" is not an std::string, cannot be used to call foo(), and the c++ compiler has to automagically rewrite 
the function call

from: int main() { foo("bar"); }
to:   int main() { foo(std::string("bar"); }

the temporary std::string object may be on the stack, but storage for text "bar" is on the heap (unless std::string 
is optimized to store short strings internally).

one can put a printf() inside foo() to print the address of xxx (should be on the stack) and xxx.c_str() (should be 
on the heap). one could also try to print the address of "bar" (should be in the read-only-constant-strings memory 
area). (I am not sure if compiler-linker combines all instances of "bar" into one, this is also easy to check).

K.O.

> 2) "advanced" c++ code:
> 
> void foo(const std::string& xxx) { ... };
> int main() { foo("bar"); }
> 
> copy-created 2nd string is avoided, but string object to hold "bar" is still must be 
> made, 1 malloc(), 1 memcpy().

Are you sure about this? I always thought that foo only receives a pointer to xxx which it puts on the stack, so
no additional malloc/free is involved.

Have a look here: https://en.cppreference.com/w/cpp/language/reference.html

It says "References are not objects; they do not necessarily occupy storage".

Stefan

> 1) history is independent from "runs", we see a change, we apply it (even if it takes 10 sec or 2 minutes).
> 
> 2) "nothing should change during a run", we must process all changes before we start a run (starting a run takes forever),
>    and we must ignore changes during a run (i.e. updated frontend starts to write new data to history). (this is why
>    the trick to "start a new run twice" used to work).

"nothing should change during a run" violates the action when a user adds a new variable during a run. So if the user does that, they don't
care if things change during a run. Then we can also modify the history DB during the run. Note that some MIDAS installations are purely
slow control (kind of a replacement of LabView, have no runs at all). In those installations runs do not make sense at all, so keeping the
history independent of runs makes sense to me.

> It is "free" to rescan ODB every 10 second or so. Then we can output a midas message "please restart the logger",
> and set an ODB flag, then when user opens the history panel editor, it will see this flag
> and tell the user "please restart the logger to see the latest changes in history". It can even list
> the specific changes, if we want ot be verbose about it.

Sounds good to me.

> I say, let's take the low road for now and see if it's good enough:
> 
> a) have the history system report any changes in midas.log - "history event added", "new history variable added" (or "renamed"),
>    this will let user see that their changes to the equipment frontend "took" and flag any accidental/unwanted changes.
> 
> b) have mlogger periodically scan ODB and set a "please restart me" flag. observe this flag in the history editor
>    and tell the user "please restart the logger to see latest changes in the history".

Actually you don't have to actively "scan" the ODB. You have hotlinks to the logger anyway from the equipment variables. All we need 
in addition is a hotline to the settings array in the ODB. The logger receives the hotline update, checks if the names changed or got
extended, then flags this as a change.

Stefan

> > Indeed. But whatever "new" we design for the scan will users complain "last week it was enough to restart the logger, now what do I have to do". So nothing 
> > is perfect. But having a button in the ODB editor like "Rebuild history database" might look more elegant. One issue is that it needs special treatment, since 
> > the logger (in the Mu3e experiment) needs >10s for the scan, so a simple rpc call will timeout.
> > 
> 
> I like the elegance of "just restart the logger".
> 
> Having a web page button to tell logger to rescan the history is cumbersome technically,
> (web page calls mjsonrpc to mhttpd, mhttpd calls a midas rpc to mlogger "please set a flag to rescan the history",
> then web page polls mhttpd to poll mlogger for "are you done yet?". or instead of polling,
> deal with double timeouts, in midas rpc to mlogger and mjsronrpc timeout in javascript).
> 
> And to avoid violating (2) above, we must tell user "you cannot push this button during a run!".
> 
> I say, let's take the low road for now and see if it's good enough:
> 
> a) have the history system report any changes in midas.log - "history event added", "new history variable added" (or "renamed"),
>    this will let user see that their changes to the equipment frontend "took" and flag any accidental/unwanted changes.
> 
> b) have mlogger periodically scan ODB and set a "please restart me" flag. observe this flag in the history editor
>    and tell the user "please restart the logger to see latest changes in the history".

This seems like a reasonable plan to me (combined with clear documentation).

Thomas

> Rather than investing time to re-invent the wheel here, better try to modify your EPICS driver process to 
become a midas process.

I am with Stefan on this. Quite a bit of work went into the tmfe c++ framework to make it easy/easier to do 
this - take an existing standalone c/c++ program and midas-ize it: in main(), "just add" calls to connect to 
midas and to start the midas threads - rpc handler, watchdog, etc.

Alternatively, one can write a midas "stdout+stderr bridge", and start your standalone program
from the programs page like this:

myprogram |& cm_msg_bridge --name "myprogram" (redirect both stdout and stderr to cm_msg_bridge stdin)

cm_msg_bridge would read stdin and put them in cm_msg(). it will connect to midas using the name "myprogram" 
to make it show "green" on the status page and it will be stoppable from the programs page.

care will need to be taken for myprogram to die cleanly when stdout and stderr are closed after cm_msg_bridge 
exits.

K.O.

> 
>   set(CMAKE_CXX_STANDARD 17)
>   set(CMAKE_CXX_STANDARD_REQUIRED ON)
>   set(CMAKE_CXX_EXTENSIONS OFF)   # optional: disables GNU extensions
> 

Looks like it works, I see -std=c++17 everywhere. Added same to manalyzer and mscb (mscb was still c++11).

Build on U-20 works (g++ accepts -std=c++17), build on CentOS-7 bombs, cmake 3.17.5 does not know CXX17.

K.O.

> > I assume that mlogger rescanning ODB is somewhat intensive process; and that's why we don't want rescanning to 
> > happen every time the ODB is changed?
> 
> A rescan maybe takes some tens of milliseconds. Something you can do on every run, but not on every ODB change (like writing to the slow control values). 
> We would need a somehow more clever code which keeps a copy of the variable names for each equipment. If the names change or the array size changes, 
> the scan can be triggered.
> 

That's right, scanning ODB for history changes is essentially free.

Question is what do we do if something was added or removed.

I see two ways to think about it:

1) history is independent from "runs", we see a change, we apply it (even if it takes 10 sec or 2 minutes).

2) "nothing should change during a run", we must process all changes before we start a run (starting a run takes forever),
   and we must ignore changes during a run (i.e. updated frontend starts to write new data to history). (this is why
   the trick to "start a new run twice" used to work).

> 
> > Stopping/restarting mlogger is okay.  But would it be better to have some alternate way to force mlogger to 
> > rescan the ODB?
>

It is "free" to rescan ODB every 10 second or so. Then we can output a midas message "please restart the logger",
and set an ODB flag, then when user opens the history panel editor, it will see this flag
and tell the user "please restart the logger to see the latest changes in history". It can even list
the specific changes, if we want ot be verbose about it.

>
> Indeed. But whatever "new" we design for the scan will users complain "last week it was enough to restart the logger, now what do I have to do". So nothing 
> is perfect. But having a button in the ODB editor like "Rebuild history database" might look more elegant. One issue is that it needs special treatment, since 
> the logger (in the Mu3e experiment) needs >10s for the scan, so a simple rpc call will timeout.
> 

I like the elegance of "just restart the logger".

Having a web page button to tell logger to rescan the history is cumbersome technically,
(web page calls mjsonrpc to mhttpd, mhttpd calls a midas rpc to mlogger "please set a flag to rescan the history",
then web page polls mhttpd to poll mlogger for "are you done yet?". or instead of polling,
deal with double timeouts, in midas rpc to mlogger and mjsronrpc timeout in javascript).

And to avoid violating (2) above, we must tell user "you cannot push this button during a run!".

I say, let's take the low road for now and see if it's good enough:

a) have the history system report any changes in midas.log - "history event added", "new history variable added" (or "renamed"),
   this will let user see that their changes to the equipment frontend "took" and flag any accidental/unwanted changes.

b) have mlogger periodically scan ODB and set a "please restart me" flag. observe this flag in the history editor
   and tell the user "please restart the logger to see latest changes in the history".

K.O.

the double-decode bug strikes again!

> This commit breaks the sequencer pages...
> 
> > Indeed a bug. Fixed in commit
> > 
> > https://bitbucket.org/tmidas/midas/commits/5c1133df073f493d74d1fc4c03fbcfe80a3edae4
> > 
> > Stefan

> This is not a bug per se, but I find it a little odd that the MVOdb functions RS, 
> RSA, RSAI, and WSA use std::string as their type, while WS ans WSAI use const 
> char*
> 
> Seems to me like simple overloading a la
> void WS(const char* varname, const std::string v, MVOdbError* error = NULL){
>     WS(varname, v.c_str(), v.size(), error);
> }
> 
> should be all that's needed, right?

No short answer to this one.

This situation is an excellent example of c++ bloat. Reduced to bare basics:

1) "naive" c++ code:

void foo(std::string xxx) { ... };
int main() { foo("bar"); }

nominally:
a new string object is created to hold "bar"
a new string object is copy-created to pass it as argument to foo()

result:
two object creations (two calls to malloc + constructors)
plus memcpy() of string data. (compiler may or may not optimize the 2nd string)

2) "advanced" c++ code:

void foo(const std::string& xxx) { ... };
int main() { foo("bar"); }

copy-created 2nd string is avoided, but string object to hold "bar" is still must be 
made, 1 malloc(), 1 memcpy().

3) "pure C" code:

void foo(const char xxx) { ... };
int main() { foo("bar"); }

address of "bar" (placed in read-only memory) is passed in a register, no malloc(), no 
memcpy(), nada, zilch.

One can argue that bloat does not matter, "just buy a bigger computer".

This ignores the fact that malloc() is quite expensive, nominally requires taking a 
mutex, and suddenly multiple threads calling foo() are unexpectedly serialized against 
the malloc() internal mutex.

I guess you can have an advanced malloc() that uses per-thread memory pools, but now 
instead of deterministic "always take a lock", we have non-deterministic "take a lock 
sometimes, when per-thread memory pools decide to jockey for more memory".

This type of non-deterministic behaviour is bad for real-time applications.

Ultimately it boils down to personal style, I prefer "C-like" efficiency and 
transparency, when I call foo() it is obvious there will be no hidden malloc(), no 
hidden mutex.

I guess mvodb could have "const std::string&" version of each "const char*" function, 
as if there is too few functions there already...

This problem is not isolated to mvodb, but pertains to any API, including midas.h.

I would say, if most function calls are foo("abc"); then "const char*" version is 
sufficient, if most calls are foo(string + "something"); then "const std::string&" is 
more appropriate.

K.O.

This commit breaks the sequencer pages...

> Indeed a bug. Fixed in commit
> 
> https://bitbucket.org/tmidas/midas/commits/5c1133df073f493d74d1fc4c03fbcfe80a3edae4
> 
> Stefan

> I assume that mlogger rescanning ODB is somewhat intensive process; and that's why we don't want rescanning to 
> happen every time the ODB is changed?

A rescan maybe takes some tens of milliseconds. Something you can do on every run, but not on every ODB change (like writing to the slow control values). 
We would need a somehow more clever code which keeps a copy of the variable names for each equipment. If the names change or the array size changes, 
the scan can be triggered.


> Stopping/restarting mlogger is okay.  But would it be better to have some alternate way to force mlogger to 
> rescan the ODB?  Like an odbedit command like 'mlogger_rescan'; or some magic ODB key to force the rescan.  I 
> guess neither of these options is really any easier for the developer.  It just seems awkward to need to restart 
> mlogger for this.

Indeed. But whatever "new" we design for the scan will users complain "last week it was enough to restart the logger, now what do I have to do". So nothing 
is perfect. But having a button in the ODB editor like "Rebuild history database" might look more elegant. One issue is that it needs special treatment, since 
the logger (in the Mu3e experiment) needs >10s for the scan, so a simple rpc call will timeout.

Let's see what KO has to say on this.

Best,
Stefan

Indeed a bug. Fixed in commit

https://bitbucket.org/tmidas/midas/commits/5c1133df073f493d74d1fc4c03fbcfe80a3edae4

Stefan

https://daq00.triumf.ca/MidasWiki/index.php/Custom_Page#modb

says that

If the ODB path does not point to an individual value but to a subdirectory, the 
whole subdirectory is mapped to this.value as a JavaSctipt object such as

<div class="modb" data-odb-path="/Runinfo" onchange="func(this.value)">

<script>function func(value) { console.log(value["run number"]); }</script>


In fact, it seems to return the JSON string of said object, so you'd have to write

console.log(JSON.parse(value)["run number"])

This is not a bug per se, but I find it a little odd that the MVOdb functions RS, 
RSA, RSAI, and WSA use std::string as their type, while WS ans WSAI use const 
char*

Seems to me like simple overloading a la
void WS(const char* varname, const std::string v, MVOdbError* error = NULL){
    WS(varname, v.c_str(), v.size(), error);
}

should be all that's needed, right?

> 3) mlogger must discover this new ODB entry:
> 
> 3a) mlogger used to rescan ODB each time something in ODB changes, this code was removed
> 3b) mlogger used to rescan ODB each time a new run is started, this code was removed
> 3c) mlogger rescans ODB each time it is restarted, this still works.
> 
> so sequence is like this: modify, restart frontend, starts a run, stop the run, observe odb entry is 
> created, restart mlogger, observe new mhf files are created in the history directory.

I assume that mlogger rescanning ODB is somewhat intensive process; and that's why we don't want rescanning to 
happen every time the ODB is changed?

Stopping/restarting mlogger is okay.  But would it be better to have some alternate way to force mlogger to 
rescan the ODB?  Like an odbedit command like 'mlogger_rescan'; or some magic ODB key to force the rescan.  I 
guess neither of these options is really any easier for the developer.  It just seems awkward to need to restart 
mlogger for this.

It would be great if mhttpd can be fixed so that it updates the cache when history editor is opened.

I switched from 

  target_compile_features(<target> PUBLIC css_std_17)

to

  set(CMAKE_CXX_STANDARD 17)
  set(CMAKE_CXX_STANDARD_REQUIRED ON)
  set(CMAKE_CXX_EXTENSIONS OFF)   # optional: disables GNU extensions

Which is now global in the CMakeLists.txt, so we only have to deal with one location if we want to change it. It also turns off the g++ options. On my
Mac I get now a clean

  -std=c++17

Please everybody test on your side. Change is committed.

Stefan