> 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

> One aspect of the MIDAS history plotting I find frustrating is the sequence for adding a new history 
> variable and then plotting them. ...

this has been a problem in MIDAS for a very long time, we have tried and failed to fix/streamline/improve 
it many times and obviously failed. many times.

this is what must happen when adding a new history variable:

1) new /eq/xxx/variables/vvv entry must show up in ODB

1a) add the code for the new data to the frontend
1b) start the frontend
1c) if new variable is added in the frontend init() method, it will be created in ODB, done.
1d) if new variable is added by the event readout code (i.e. via MIDAS event data bank automatically 
written to ODB by RO_ODB flags), then we need to start a run.
1e) if this is not periodic event, but beam event or laser event or some other triggered event, we must 
also turn on the beam, turn on the laser, etc.

1z) observe that ODB entry exists

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.

4) mhttpd must discover that a new mhf file now exists, read it's header to discover history event and 
variable names and make them available to the history panel editor.

it is not clear to me that this part currently works:

4a) mhttpd caches the history event list and will not see new variables unless this cache is updated.
4b) when web history panel editor is opened, it is supposed to tell mhttpd to update the cache. I am 
pretty sure it worked when I wrote this code...
4c) but obviously it does not work now.

restarting mhttpd obviously makes it load the history data anew, but there is no button to make it happen 
on the MIDAS web pages.

so it sounds like I have to sit down and at least retest this whole scheme to see that it works at least 
in some way.

then try to improve it:

a) the frontend dance in (1) is unavoidable
b) mlogger must be restarted, I think Stefan and myself agree on this. In theory we could add a web page 
button to call an mlogger RPC and have it reload the history. but this button already exists, it's called 
"restart mlogger".
c) newly create history event should automatically show up in the history panel editor without any 
additional user action
d) document the two intermediate debugging steps:
d1) check that the new variable was created in ODB
d2) check that mlogger created (and writes to) the new history file

this is how I see it and I am open to suggestion, changes, improvements, etc.

K.O.

Hi, Jonas, thank you for reminding me about this. I hope to work on manalyzer in the next few weeks and I will review the ROOT output file name scheme.

K.O.



> Hi all,
> 
> Could you please get back to me about whether something like my earlier suggestion might be considered, or if I should set up some workaround to rename files at EOR for our experiments?  
> 
> https://daq00.triumf.ca/elog-midas/Midas/3042 :
> -----------------------------------------------
> > Hi all,
> > 
> > Would it be possible to extend manalyzer to support custom .root file names that include the run number? 
> > 
> > As far as I understand, the current behavior is as follows:
> > The default filename is ./root_output_files/output%05d.root , which can be customized by the following two command line arguments.
> > 
> > -Doutputdirectory: Specify output root file directory
> > -Ooutputfile.root: Specify output root file filename
> > 
> > If an output file name is specified with -O, -D is ignored, so the full path should be provided to -O. 
> > 
> > I am aiming to write files where the filename contains sufficient information to be unique (e.g., experiment, year, and run number). However, if I specify it with -O, this would require restarting manalyzer after every run; a scenario that I would like to avoid if possible.
> > 
> > Please find a suggestion of how manalyzer could be extended to introduce this functionality through an additional command line argument at
> > https://bitbucket.org/krieger_j/manalyzer/commits/24f25bc8fe3f066ac1dc576349eabf04d174deec
> > 
> > Above code would allow the following call syntax: ' ./manalyzer.exe -O/data/experiment1_%06d.root --OutputNumbered '
> > But note that as is, it would fail if a user specifies an incompatible format such as -Ooutput%s.root . 
> > 
> > So a safer, but less flexible option might be to instead have the user provide only a prefix, and then attach %05d.root in the code.
> > 
> > Thank you for considering these suggestions!

> target_compile_features(<target> PUBLIC cxx_std_17)
> which correctly causes a
> c++ -std=gnu++17 ...

I think this is set in a couple of places, yet I do not see any -std=xxx flag passed to the compiler.

(and I am not keen on spending a full day fighting cmake)

(btw, -std=c++17 and -std=gnu++17 are not the same thing, I am not sure how well GNU extensions are supported on 
macos)

K.O.

> 
> > I notice the cmake does not actually pass "-std=c++17" to the c++ compiler, and on U-20, it is likely 
> > the default c++14 is used. cmake always does the wrong thing and this will need to be fixed later.
> 
> set(CMAKE_CXX_STANDARD 17)
> set(CMAKE_CXX_STANDARD_REQUIRED ON)
>

We used to have this, it is not there now.

> 
> Get it to do the right thing?
> 

Unlikely, as Stefan reported, asking for C++17 yields -std=gnu++17 which is close enough, but not the same 
thing.

For now, it does not matter, U-22 and U-24 are c++17 by default, if somebody accidentally commits c++20 
code, builds will fail and somebody will catch it and complain, plus the weekly bitbucket build will bomb-
out.

On U-20, default is c++14 and builds will start bombing out as soon as we commit some c++17 code.

el7 builds have not worked for some time now (a bizarre mysterious error)

el8, el9, el10 likely same situation as Ubuntu.

macos, not sure.

K.O.

> Thanks --- it looks like this commit resolves the issue for us ...
> Thanks to Ben Smith for pointing us at exactly the right commit

I would like to take the opportunity to encourage all to report bug fixes like this one to this mailing list.

This looks like a serious bug, many midas users would like to know when it was introduced, when found, when fixed 
and who takes the credit.

K.O.

> Thanks for the fixes, which I all approve.
> 
> There is still a "follow_links" in midas_c_compat.h line 70 for Python. Probably Ben has to look into that. Also 
> client.py has it.

Correct, Ben will look at this on the python side.

And I will be updating mvodb soon and fix it there.

K.O.

Function db_find_keys() added by person unnamed in April 2020 never worked correctly, it is now fixed, 
repaired, also unsafe strcpy() replaced by mstrlcpy().

This function is used by msequencer ODBSet function and by odbedit "set" command.

Under all conditions it returned DB_NO_KEYS, only two use cases actually worked:

set runinfo/state 1 <--- no match pattern - works
set run*/state 1    <--- match multiple subdirectories - works
set runinfo/stat* 1 <--- bombs out with DB_NO_KEY
set run*/stat* 1    <--- bombs out with DB_NO_KEY

All four use cases now work.

commit b5b151c9bc174ca5fd71561f61b4288c40924a1a

K.O.

Thanks for the fixes, which I all approve.

There is still a "follow_links" in midas_c_compat.h line 70 for Python. Probably Ben has to look into that. Also 
client.py has it.

Stefan