Back Midas Rome Roody Rootana
  Midas DAQ System, Page 120 of 121  Not logged in ELOG logo
ID Date Author Topicup Subject
  2179   28 May 2021 Stefan RittSuggestionHave a list of 'users responsible' in Alarms and Programs odb entries
I think this is a good idea and I support it. We have a similar problem in MEG and 
we solved that with external (bash) scripts called in case of alarms. One feature 
there we have is that for some alarms, several people want to get notified. So 
people can "subscribe" to certain alarms. The subscription are now handled inside 
Slack which I like better, but maybe it would be good to have more than one "user 
responsible". Like if one person is sleeping/traveling, it's good to have a 
substitute. Can you make an array out of that? Or a comma-separated list?

Best,
Stefan
  2182   28 May 2021 Joseph McKennaSuggestionHave a list of 'users responsible' in Alarms and Programs odb entries
> I think this is a good idea and I support it. We have a similar problem in MEG and 
> we solved that with external (bash) scripts called in case of alarms. One feature 
> there we have is that for some alarms, several people want to get notified. So 
> people can "subscribe" to certain alarms. The subscription are now handled inside 
> Slack which I like better, but maybe it would be good to have more than one "user 
> responsible". Like if one person is sleeping/traveling, it's good to have a 
> substitute. Can you make an array out of that? Or a comma-separated list?
> 
> Best,
> Stefan

Presently there are 256 characters in the 'users responsible' field, so you can just 
list many users (no space, space or comma whatever). Discord, slack and mattermost 
don't care, they just parse the user tags.

I can still make this an array and pass a std::vector<std::string> into 
al_trigger_class function?
  2183   28 May 2021 Stefan RittSuggestionHave a list of 'users responsible' in Alarms and Programs odb entries
> I can still make this an array and pass a std::vector<std::string> into 
> al_trigger_class function?

Maybe 256 chars are enough at the moment. If other people complain in the future, we can 
re-visit.

Stefan
  2184   28 May 2021 Joseph McKennaSuggestionHave a list of 'users responsible' in Alarms and Programs odb entries
> > I can still make this an array and pass a std::vector<std::string> into 
> > al_trigger_class function?
> 
> Maybe 256 chars are enough at the moment. If other people complain in the future, we can 
> re-visit.
> 
> Stefan

Thinking about it, an array of maybe 80 character would give enough space for a name, a tag 
and phone number. Do I need to budget memory very strictly? Would 32 entries of 80 
characters be too much? 
  2188   28 May 2021 Stefan RittSuggestionHave a list of 'users responsible' in Alarms and Programs odb entries
> > > I can still make this an array and pass a std::vector<std::string> into 
> > > al_trigger_class function?
> > 
> > Maybe 256 chars are enough at the moment. If other people complain in the future, we can 
> > re-visit.
> > 
> > Stefan
> 
> Thinking about it, an array of maybe 80 character would give enough space for a name, a tag 
> and phone number. Do I need to budget memory very strictly? Would 32 entries of 80 
> characters be too much? 

On that level memory is cheap.

Stefan
  2189   28 May 2021 Joseph McKennaSuggestionHave a list of 'users responsible' in Alarms and Programs odb entries

I've updated the branch / pull request to use an array of 10 entries (80 chars each). 32 felt a 
little overkill when I saw it on screen, but absolutely happy to set it to any number you 
recommend.

The array gets flattened out when an alarm is triggered, currently the formatting produces

AlarmClass : AlarmMessage (Flattened List Of Users Responsible Array With Space Separators)

If experiments want to use Discord / Slack / Mattermost tags and or add phone numbers, that 
should fit in 80 characters
  2191   31 May 2021 Joseph McKennaSuggestionHave a list of 'users responsible' in Alarms and Programs odb entries
This list of responsible being attached to alarm message strings will be great for the 
mmessenger, however, perhaps its going to generate very long messages for the speaker programs 
(web interface and mlxspeaker ):

AlarmClass : AlarmMessage (ResponsibleUser1 ResponsibleUser2 ResponsibleUser3 ResponsibleUser4 
... ResponsibleUser4)

especially if people put in user tags or emergency contact details...

Should we add a key word or character for the programs that create audio to parse that silence 
the list of responsible users? I'd be tempted to use a single character but there is a risk 
users might have that in a custom alarm message. Maybe something usual like the 'bel' 
character? '|'? 

Perhaps use the string 'Responsible:' or 'Users:' to trim out the Users Responsible list from 
the message string?

AlarmClass : AlarmMessage Responsible:(ResponsibleUser1 ResponsibleUser2 ResponsibleUser3 
ResponsibleUser4 ... ResponsibleUser4)

AlarmClass : AlarmMessage Users:(ResponsibleUser1 ResponsibleUser2 ResponsibleUser3 
ResponsibleUser4 ... ResponsibleUser4)
  2195   02 Jun 2021 Konstantin OlchanskiSuggestionHave a list of 'users responsible' in Alarms and Programs odb entries
> This list of responsible being attached to alarm message strings ...

This is a great idea. But I think we do not need to artificially limit ourselves
to string and array lengths.

The code in alarm.c should be changes to use std::string and std::vector<std::string> (STRING_LIST 
#define), db_get_record() should be replaced with individual ODB reads (that's what it does behind 
the scenes, but in a non-type and -size safe way).

I think the web page code will work correctly, it does not care about string lengths.

K.O.
  Draft   08 Jun 2021 Joseph McKennaSuggestionHave a list of 'users responsible' in Alarms and Programs odb entries
> > This list of responsible being attached to alarm message strings ...
> 
> This is a great idea. But I think we do not need to artificially limit ourselves
> to string and array lengths.
> 
> The code in alarm.c should be changes to use std::string and std::vector<std::string> (STRING_LIST 
> #define), db_get_record() should be replaced with individual ODB reads (that's what it does behind 
> the scenes, but in a non-type and -size safe way).
> 
> I think the web page code will work correctly, it does not care about string lengths.
> 
> K.O.

Ok, I'm working on this... I see a design choice to make, 1. Keep 'ALARM' as a struct, or 2. Replace ALARM struct with a class (keeping memory layout the same). Since we are adding STRING_LIST, I'd lean towards a C++ style with a class

1. Keep 'ALARM' as a struct:
Get rid of ALARM_ODB_STR for the default values
Add static functions (that take a hkey) to interact with the ODB (to save duplicating logic in al_reset_alarm, al_check, al_define_odb_alarm and al_trigger_alarm)

2. Replace ALARM struct with a class:
default ctor: Do nothing special (so it behaves like the old struct)
default dtor: std::vector<std::string> dtor will

It seems an opportunity to convert the alarm struct to a class with member functions that take hkey pointers?

default ctor: Do nothing special (so it behaves like the old struct)
default dtor: std::vector<std::string> dtor will
SetToDefault: This is where default values are hard coded (functionally replace ALARM_ODB_STR )
  2212   09 Jun 2021 Joseph McKennaSuggestionHave a list of 'users responsible' in Alarms and Programs odb entries
> > This list of responsible being attached to alarm message strings ...
> 
> This is a great idea. But I think we do not need to artificially limit ourselves
> to string and array lengths.
> 
> The code in alarm.c should be changes to use std::string and std::vector<std::string> (STRING_LIST 
> #define), db_get_record() should be replaced with individual ODB reads (that's what it does behind 
> the scenes, but in a non-type and -size safe way).
> 
> I think the web page code will work correctly, it does not care about string lengths.
> 
> K.O.

Auto growing lists is an excellent plan. I am making decent progress and should have something to 
report soon
  Draft   16 Jun 2021 Joseph McKennaSuggestionHave a list of 'users responsible' in Alarms and Programs odb entries
> > > This list of responsible being attached to alarm message strings ...
> > 
> > This is a great idea. But I think we do not need to artificially limit ourselves
> > to string and array lengths.
> > 
> > The code in alarm.c should be changes to use std::string and std::vector<std::string> (STRING_LIST 
> > #define), db_get_record() should be replaced with individual ODB reads (that's what it does behind 
> > the scenes, but in a non-type and -size safe way).
> > 
> > I think the web page code will work correctly, it does not care about string lengths.
> > 
> > K.O.
> 
> Auto growing lists is an excellent plan. I am making decent progress and should have something to 
> report soon

This has sent me down a little rabbit hole, and I'd like to check in with efforts to improve the efficiency and simplicity of the alarm code.

I can keep with the current 'C' style of the alarm.cxx code, replace struct read and writes to the odb with individual odb entries... put functions in alarm.cxx to create, read and write to the odb... 

If we go this 'C' style route, then I'll have duplication of the 'users responsible' setters and getter functions for structs ALARM and PROGRAM_INFO

What would the MIDAS developers thing of creating classes for ALARM and PROGRAM_INFO (I am thinking for binary compatibilities of not touching ALARM and PROGRAM_INFO structs, and inheriting from them:

class UsersResponsible
{
   public:
      STRING_LIST fUsersResponsible;
}

class Alarm: public ALARM, public UsersResponsible
{
}

class ProgramInfo: public PROGRAM_INFO, public UsersResponsible
{
}

Each of these three classes would have member to functions to Create, Read and Write to the ODB. We could get rid of the PROGRAM_INFO_STR precompiler macro and instead have a SetToDefault member function. 

It seems clear we should set the ODB path in the constructor of Alarm and ProgramInfo
  2225   20 Jun 2021 Lukas GerritzenSuggestionMidasConfig.cmake usage
I agree that those two things are problems, but I don't see why it is preferable to leave the MidasConfig.cmake in this "broken" state. For us 
problem 1 is less of an issue, becaues we run "link_directories(${MIDAS_LIBRARY_DIRS})" in the top CMakeLists.txt and then just link against "midas", 
not "${MIDAS_LIBRARIES}". However, number 2 would be nice, to not manually hack in target_include_directories(target ${MIDASSYS}/mscb/include), 
especially because ${MIDASSYS} is not set in cmake. 

I see two solutions for problem 2: Treat mscb as a submodule and compile and install it together with midas, or add the include directory to 
${MIDAS_INCLUDE_DIRS} (same applies to the other submodules, mscb is the one that made me open this elog just now)

Cheers
Lukas
 
> > find_package(Midas)
> 
> I am testing find_package(Midas). There is a number of problems:
> 
> 1) ${MIDAS_LIBRARIES} is set to "midas;midas-shared;midas-c-compat;mfe".
> 
> This seem to be an incomplete list of all libraries build by midas (rmana is missing).
> 
> This means ${MIDAS_LIBRARIES} should not be used for linking midas programs (unlike ${ROOT_LIBRARIES}, etc):
> 
> - we discourage use of midas shared library because it always leads to problems with shared library version mismatch (static linking is preferred)
> - midas-c-compat is for building python interfaces, not for linking midas programs
> - mfe contains a main() function, it will collide with the user main() function
> 
> So I think this should be changed to just "midas" and midas linking dependancy
> libraries (-lutil, -lrt, -lpthread) should also be added to this list.
> 
> Of course the "install(EXPORT)" method does all this automatically. (so my fixing find_package(Midas) is a waste of time)
> 
> 2) ${MIDAS_INCLUDE_DIRS} is missing the mxml, mjson, mvodb, midasio submodule directories
> 
> Again, install(EXPORT) handles all this automatically, in find_package(Midas) it has to be done by hand.
> 
> Anyhow, this is easy to add, but it does me no good in the rootana cmake if I want to build against old versions
> of midas. So in the rootana cmake, I still have to add $MIDASSYS/mvodb & co by hand. Messy.
> 
> I do not know the history of cmake and why they have two ways of doing things (find_package and install(EXPORT)),
> this second method seems to be much simpler, everything is exported automatically into one file,
> and it is much easier to use (include the export file and say target_link_libraries(rootana PUBLIC midas)).
> 
> So how much time should I spend in fixing find_package(Midas) to make it generally usable?
> 
> - include path is incomplete
> - library list is nonsense
> - compiler flags are not exported (we do not need -DOS_LINUX, but we do need -DHAVE_ZLIB, etc)
> - dependency libraries are not exported (-lz, -lutil, -lrt, -lpthread, etc)
> 
> K.O.
  2226   20 Jun 2021 Konstantin OlchanskiSuggestionMidasConfig.cmake usage
> I agree that those two things are problems, but I don't see why it is preferable to leave the MidasConfig.cmake in this "broken" state. For us 
> problem 1 is less of an issue, becaues we run "link_directories(${MIDAS_LIBRARY_DIRS})" in the top CMakeLists.txt and then just link against "midas", 
> not "${MIDAS_LIBRARIES}". However, number 2 would be nice, to not manually hack in target_include_directories(target ${MIDASSYS}/mscb/include), 
> especially because ${MIDASSYS} is not set in cmake.

So you say "nuke ${MIDAS_LIBRARIES}" and "fix ${MIDAS_INCLUDE}". Ok.

Problem still remains with required auxiliary libraries for linking "-lmidas". Sometimes you
need "-lutil" and "-lrt" and "-lpthread", sometimes not. Some way to pass this information
automatically would be nice.

Problem still remains that I cannot do these changes because I have no test harness
for any of this. Would be great if you could contribute this and post the documentation
blurb that we can paste into the midas wiki documentation.

And I still do not understand why we have to do all this work when cmake "import(EXPORT)"
already does all of this automatically. What am I missing?

K.O.

> 
> I see two solutions for problem 2: Treat mscb as a submodule and compile and install it together with midas, or add the include directory to 
> ${MIDAS_INCLUDE_DIRS} (same applies to the other submodules, mscb is the one that made me open this elog just now)
> 
> Cheers
> Lukas
>  
> > > find_package(Midas)
> > 
> > I am testing find_package(Midas). There is a number of problems:
> > 
> > 1) ${MIDAS_LIBRARIES} is set to "midas;midas-shared;midas-c-compat;mfe".
> > 
> > This seem to be an incomplete list of all libraries build by midas (rmana is missing).
> > 
> > This means ${MIDAS_LIBRARIES} should not be used for linking midas programs (unlike ${ROOT_LIBRARIES}, etc):
> > 
> > - we discourage use of midas shared library because it always leads to problems with shared library version mismatch (static linking is preferred)
> > - midas-c-compat is for building python interfaces, not for linking midas programs
> > - mfe contains a main() function, it will collide with the user main() function
> > 
> > So I think this should be changed to just "midas" and midas linking dependancy
> > libraries (-lutil, -lrt, -lpthread) should also be added to this list.
> > 
> > Of course the "install(EXPORT)" method does all this automatically. (so my fixing find_package(Midas) is a waste of time)
> > 
> > 2) ${MIDAS_INCLUDE_DIRS} is missing the mxml, mjson, mvodb, midasio submodule directories
> > 
> > Again, install(EXPORT) handles all this automatically, in find_package(Midas) it has to be done by hand.
> > 
> > Anyhow, this is easy to add, but it does me no good in the rootana cmake if I want to build against old versions
> > of midas. So in the rootana cmake, I still have to add $MIDASSYS/mvodb & co by hand. Messy.
> > 
> > I do not know the history of cmake and why they have two ways of doing things (find_package and install(EXPORT)),
> > this second method seems to be much simpler, everything is exported automatically into one file,
> > and it is much easier to use (include the export file and say target_link_libraries(rootana PUBLIC midas)).
> > 
> > So how much time should I spend in fixing find_package(Midas) to make it generally usable?
> > 
> > - include path is incomplete
> > - library list is nonsense
> > - compiler flags are not exported (we do not need -DOS_LINUX, but we do need -DHAVE_ZLIB, etc)
> > - dependency libraries are not exported (-lz, -lutil, -lrt, -lpthread, etc)
> > 
> > K.O.
  2228   22 Jun 2021 Lukas GerritzenSuggestionMidasConfig.cmake usage
> So you say "nuke ${MIDAS_LIBRARIES}" and "fix ${MIDAS_INCLUDE}". Ok.

A more moderate option would be to remove mfe from ${MIDAS_LIBRARIES}, but as far as I understand mfe is not the only problem, so nuking might be the 
better option after all. In addition, setting ${MIDASSYS} in MidasConfig.cmake would probably improve compatibility.

>Sometimes you need "-lutil" and "-lrt" and "-lpthread", sometimes not. 
>Some way to pass this information automatically would be nice.

I do not properly understand when you need this and when not, but can't this be communicated with the PUBLIC keyword of target_link_libraries()? If I 
understand if we can use PUBLIC for -lutil, -lrt and -lpthread, I can write something, test it here and create a pull request.

> And I still do not understand why we have to do all this work when cmake "import(EXPORT)"
> already does all of this automatically. What am I missing?

Does this not require midas to be built every time you import it? I know, it's a bit the "billions of flies can't be wrong" argument, but I've never seen 
any package that uses import(EXPORT) over find_package().

> > I agree that those two things are problems, but I don't see why it is preferable to leave the MidasConfig.cmake in this "broken" state. For us 
> > problem 1 is less of an issue, becaues we run "link_directories(${MIDAS_LIBRARY_DIRS})" in the top CMakeLists.txt and then just link against "midas", 
> > not "${MIDAS_LIBRARIES}". However, number 2 would be nice, to not manually hack in target_include_directories(target ${MIDASSYS}/mscb/include), 
> > especially because ${MIDASSYS} is not set in cmake.
> 
> So you say "nuke ${MIDAS_LIBRARIES}" and "fix ${MIDAS_INCLUDE}". Ok.
> 
> Problem still remains with required auxiliary libraries for linking "-lmidas". Sometimes you
> need "-lutil" and "-lrt" and "-lpthread", sometimes not. Some way to pass this information
> automatically would be nice.
> 
> Problem still remains that I cannot do these changes because I have no test harness
> for any of this. Would be great if you could contribute this and post the documentation
> blurb that we can paste into the midas wiki documentation.
> 
> And I still do not understand why we have to do all this work when cmake "import(EXPORT)"
> already does all of this automatically. What am I missing?
> 
> K.O.
> 
> > 
> > I see two solutions for problem 2: Treat mscb as a submodule and compile and install it together with midas, or add the include directory to 
> > ${MIDAS_INCLUDE_DIRS} (same applies to the other submodules, mscb is the one that made me open this elog just now)
> > 
> > Cheers
> > Lukas
> >  
> > > > find_package(Midas)
> > > 
> > > I am testing find_package(Midas). There is a number of problems:
> > > 
> > > 1) ${MIDAS_LIBRARIES} is set to "midas;midas-shared;midas-c-compat;mfe".
> > > 
> > > This seem to be an incomplete list of all libraries build by midas (rmana is missing).
> > > 
> > > This means ${MIDAS_LIBRARIES} should not be used for linking midas programs (unlike ${ROOT_LIBRARIES}, etc):
> > > 
> > > - we discourage use of midas shared library because it always leads to problems with shared library version mismatch (static linking is preferred)
> > > - midas-c-compat is for building python interfaces, not for linking midas programs
> > > - mfe contains a main() function, it will collide with the user main() function
> > > 
> > > So I think this should be changed to just "midas" and midas linking dependancy
> > > libraries (-lutil, -lrt, -lpthread) should also be added to this list.
> > > 
> > > Of course the "install(EXPORT)" method does all this automatically. (so my fixing find_package(Midas) is a waste of time)
> > > 
> > > 2) ${MIDAS_INCLUDE_DIRS} is missing the mxml, mjson, mvodb, midasio submodule directories
> > > 
> > > Again, install(EXPORT) handles all this automatically, in find_package(Midas) it has to be done by hand.
> > > 
> > > Anyhow, this is easy to add, but it does me no good in the rootana cmake if I want to build against old versions
> > > of midas. So in the rootana cmake, I still have to add $MIDASSYS/mvodb & co by hand. Messy.
> > > 
> > > I do not know the history of cmake and why they have two ways of doing things (find_package and install(EXPORT)),
> > > this second method seems to be much simpler, everything is exported automatically into one file,
> > > and it is much easier to use (include the export file and say target_link_libraries(rootana PUBLIC midas)).
> > > 
> > > So how much time should I spend in fixing find_package(Midas) to make it generally usable?
> > > 
> > > - include path is incomplete
> > > - library list is nonsense
> > > - compiler flags are not exported (we do not need -DOS_LINUX, but we do need -DHAVE_ZLIB, etc)
> > > - dependency libraries are not exported (-lz, -lutil, -lrt, -lpthread, etc)
> > > 
> > > K.O.
  2230   24 Jun 2021 Konstantin OlchanskiSuggestionMidasConfig.cmake usage
> > So you say "nuke ${MIDAS_LIBRARIES}" and "fix ${MIDAS_INCLUDE}". Ok.
> A more moderate option ...

For the record, I did not disappear. I have a very short time window
to complete commissioning the alpha-g daq (now that the network
and the event builder are cooperating). To add to the fun, our high voltage
power supply turned into a pumpkin, so plotting voltages and currents 
on the same history plot at the same time (like we used to be able to do)
went up in priority. 

K.O.
  2240   28 Jun 2021 Marco FrancesconiSuggestionODB Load in Sequencer
Hi all,
for my experiment we ended up with the need of changing lot of parameters (~9000 values) in the ODB at once by the sequencer.
The very first solution was to use a sequencer function with a ton of ODBSET calls, however a more elegant solution may be to provide an "ODBLoad" command which mimics the "load" command of odbedit.
I already have a working modification to the sequencer for this, if you agree I will commit it to a dedicated brach.
Let me know if you think this is a good approach.

Marco F
  2241   28 Jun 2021 Stefan RittSuggestionODB Load in Sequencer
> Hi all,
> for my experiment we ended up with the need of changing lot of parameters (~9000 values) in the ODB at once by the sequencer.
> The very first solution was to use a sequencer function with a ton of ODBSET calls, however a more elegant solution may be to provide an "ODBLoad" command which mimics the "load" command of odbedit.
> I already have a working modification to the sequencer for this, if you agree I will commit it to a dedicated brach.
> Let me know if you think this is a good approach.
> 
> Marco F

How can people judge your modification if they cannot see it? Why don't you make a pull request, so it can be properly reviewed.

Stefan
  2242   28 Jun 2021 Konstantin OlchanskiSuggestionODB Load in Sequencer
> Hi all,
> for my experiment we ended up with the need of changing lot of parameters (~9000 values) in the ODB at once by the sequencer.
> The very first solution was to use a sequencer function with a ton of ODBSET calls, however a more elegant solution may be to provide an "ODBLoad" command which mimics the "load" command of odbedit.
> I already have a working modification to the sequencer for this, if you agree I will commit it to a dedicated brach.
> Let me know if you think this is a good approach.
> 

Sounds like a good idea. I trust you are using the data in json format? Perhaps the command
should be named "ODBLoadJSON" to be clear about this.

(JSON is preferred over .odb and .xml for many reasons (ask me))

K.O.
  2243   28 Jun 2021 Stefan RittSuggestionODB Load in Sequencer
> > Hi all,
> > for my experiment we ended up with the need of changing lot of parameters (~9000 values) in the ODB at once by the sequencer.
> > The very first solution was to use a sequencer function with a ton of ODBSET calls, however a more elegant solution may be to provide an "ODBLoad" command which mimics the "load" command of odbedit.
> > I already have a working modification to the sequencer for this, if you agree I will commit it to a dedicated brach.
> > Let me know if you think this is a good approach.
> > 
> 
> Sounds like a good idea. I trust you are using the data in json format? Perhaps the command
> should be named "ODBLoadJSON" to be clear about this.
> 
> (JSON is preferred over .odb and .xml for many reasons (ask me))

What if some experiment keep some files in .xml format (ask me!). The routine should check for the extension and support all three formats.

Stefan
  2244   28 Jun 2021 Konstantin OlchanskiSuggestionODB Load in Sequencer
> > > Hi all,
> > > for my experiment we ended up with the need of changing lot of parameters (~9000 values) in the ODB at once by the sequencer.
> > > The very first solution was to use a sequencer function with a ton of ODBSET calls, however a more elegant solution may be to provide an "ODBLoad" command which mimics the "load" command of odbedit.
> > > I already have a working modification to the sequencer for this, if you agree I will commit it to a dedicated brach.
> > > Let me know if you think this is a good approach.
> > > 
> > 
> > Sounds like a good idea. I trust you are using the data in json format? Perhaps the command
> > should be named "ODBLoadJSON" to be clear about this.
> > 
> > (JSON is preferred over .odb and .xml for many reasons (ask me))
> 
> What if some experiment keep some files in .xml format (ask me!). The routine should check for the extension and support all three formats.
> 

Yes, hard to tell without seeing his full proposal, including the code. If it is load from file,
sure we look at the file extension, I think the existing code already would do this and support all 3 formats.

But if he wants to load ODB data from a text literal or from a string,
we might as well stick to json. I guess we could support the other formats, but I do not see anybody
using anything other than json for new code like this.

ODBPasteJSON("/foo/bar/baz", '{"var1":1, "var2":"somestr"}');

K.O.
ELOG V3.1.4-2e1708b5