Good day to all of you,
I made my first (very simple) attempt at Actor Framework and build an acquisition system. I was wondering if any of you would be so kind to read my code and make a brief review of the good and the bad thing I made during development.
In particular if I have understand correctly the helper loop use, the comunication beetwen the actor and the helper loop and the system to stop it.
Thank you very much,
You need to make a virtual DaqMX Device named "DevTest" in order to run the VI.
Just my opinion, but I don't like that you have the DAQ Task being used in so many different places - its an easy way to create race conditions. For example in your DAQ Actor 'Reset' method you are stopping and clearing the DAQ task, while the Reader actor could be actively using that task, and could in fact have many messages left in its queue telling it to read from a now dead DAQ task.
I am also not quite sure why you are using the helper loop in the Reader actor - I would get rid of it completely (and in fact probably get rid of the reader actor all together and just do all the DAQ stuff in the DAQ actor.) In fact, I think you can probably get rid of the helper loop in the DAQ actor as well - in my opinion here it just adds unnecessary complexity. For example, your reader actor reads a value, and passes it to the DAQ actor. The DAQ actor then passes that value to its helper loop, which then passes it up to the UI. This could be greatly simplified by having the DAQ actor read in a method called in actor core, then directly send the message to the UI.
(What I would do) - When you want to start periodic reading, just use a time delayed message to send messages to the DAQ actor to read. Put an 'Reading Active' boolean in your DAQ actor's class data, and set it to true when you start the periodic messages. In your read method, check that boolean before doing a read, and set it back to false when you stop reading. This way, if a read message is enqueued while the stop reading is called, it won't attempt to read from a dead task. (Keep the time delayed message notifier in class data as well so you can stop the messaging.)
Thank you Paul this is right the constructive criticism that I need.
Yes, you're completely right on the Reader Actor, i just wanted to create one more actor to train me a little more on the creation procedure but it is completly a waste of bytes. In my first version all the reading code was in the DAQ Actor.
Still there is one thing i have'nt understand. You said that even the helper loop on the Daq Actor is useless, but without it how can I timing the reading task?
I wouldn't necessarily say the helper loop is useless, just in this case, I personally probably wouldn't do it that way - not saying its necessarily wrong.
To get the periodic reads, you can use the time delayed send message - its in the actor framework palette under advanced. This VI launches another VI that runs in the background and will send your actor messages at a pre defined interval. When you call the method, it returns a notifier reference that you can use to stop the messaging. (Or skip the next scheduled message/send the next message now)
So in your case, I would set it up to send the read message at a pre defined interval once you have commanded the actor to start reading. You just need to ensure that the actor will be able to process those incoming messages as quickly as they are being sent, otherwise the actor's queue will fill up and you will end up with a memory leak.
Another possible solution is to use your own helper loop as a timer - after each read, message the helper loop to start the timer, and when the timeout occurs, send the message to read again. The helper loop then goes back to just waiting until the next read is complete. This eliminates the concern of read messages filling up the actor's queue, however it does involve a bit more coding.
An argument could be made that the memory leak concern is good reason to leave the reading in the helper loop, and use the queue timeout to do the periodic reading - and you will probably find others who suggest that. With either solution, I recommend trying to keep the helper loop clean and single purposed - if its a DAQ helper loop, keep it for DAQ stuff and DAQ stuff only. If youre using it as timer, use it for that and that alone.
Thanks Paul, your suggestions are enlightening. Now the code is clearly cleaner and more readable.
Now if I want to implement a good way to capture and handle the error, what is the best strategy? I was thinking of sending back the error to the Root Actor and, from the Root Actor, launching an Error Actor that will handle the whole thing. This is to avoid horizontal communication between actors.
I don't think there is any particular best strategy - you will find lots of differing opinions on that subject, and it really depends on how complex your project is/how far you want to take it. Generally speaking, I don't really like the idea of a single error handler actor, unless it is just there to display errors to the user/log them in a well defined way. I think each actor should at least to some degree, handle its own errors.
For example, the DAQ functions can throw all sorts of errors, each of which may have a different required response. If you put the handling in a single error handler, that error handler is pretty tightly coupled to the DAQ actor. In your application as it stands, that may not be a problem. But what if you would like to create a hardware abstraction, and may use hardware other than your DAQ device? So you will need to update your error handler to know how to handle any errors thrown by each new type of hardware. If you add other system components like a data logger, you will also have to update the error handler to know what to do with these types of errors - the error handler will grow to be a pretty big mess fairly quickly.
Personally, I have each actor in my code log its errors (using an event logging API all actors have access to), then determine what to do on an actor by actor basis.
Just make sure to do something for error handling, if it is just using the general error handler to display a dialog to the user. Few things are as annoying or as unprofessional as an application that just closes with no indication as to why (default actor behavior on an error.) Also make sure you clear the error out of handle error if you don't stop the actor when an error occurs, as if you don't you won't have a way to stop the actor as the dequeue no longer executes.
The event logger? Sure, that is essentially what mine is - actually an FGV of an actor's enqueuer. The initialize event logger method launches an actor and then stores it's enqueuer in an FGV, and calls to log just package up a log message and send it to the actor.
If you wanted to build an error handler actor that just formatted errors in a pretty way to display to the user and offered generic actions on what to do next, I don't see anything wrong with sharing it's enqueuer through an FGV (I wouldn't give direct access to the queue, but rather allow a defined 'process this error' message to be called) Just be mindful of your coupling - all your actors will then depend on that error handler api, so make sure that the error handler has no application specific dependencies.
In reference to "Just make sure to do something for error handling, if it is just using the general error handler to display a dialog to the user." when i need to manage the error inside the "Handle Error.vi" i must do it before the "Call Parent Method" to intercept and clean the error and avoiding the shutdown of the Actor, am I right?
You don't need to do it before the call parent method, that is up to you. 'Handle Error' has an output Boolean, 'Stop Actor?' - that is what is used to determine whether or not to stop the actor. The parent method by default always returns a true, and if the error is error code 43, it clears the error out. (Error code 43 is used to stop an actor)
So, one thing you could do is call the parent method, and check the output error - if it outputs an error, a real error occurred and you should do something with it. If the call parent method doesnt output an error, it means the actor was sent a stop message and you should stop it. If you aren't going to stop the actor when a real error does occur, make sure you clear the final output error - it will go onto a shift register, and prevent the actor from receiving any further messages.