API review
Proposer: Tully Foote
Present at review:
- Josh
- Tully
- Melonee
- Sachin
Question / concerns / comments
Enter your thoughts on the API and any questions / concerns you have here. Please sign your name. Anything you want to address in the API review should be marked down here before the start of the meeting.
Josh:
In general, anything implicitly allocated through a function (ie. FilterFactory::CreateObject) should be freed through a function in the library as well, rather than just deleting it. I'm not sure if Loki::Factory supports this.
Base Class and Factory Definition
/** \todo there's got to be a better way to do this. */
template <typename T>
std::string getTypeString();
template <>
std::string getTypeString<float>() { return std::string("<float>");};
template <>
std::string getTypeString<double>() { return std::string("<double>");};
/** \brief A Base filter class to provide a standard interface for all filters
*
*/
template<typename T>
class FilterBase
{
public:
FilterBase(){};
virtual ~FilterBase(){};
virtual bool configure(unsigned int number_of_elements, const std::string & arguments)=0;
/** \brief Update the filter and return the data seperately
*/
virtual bool update(const T* const data_in, T* data_out)=0;
std::string getType() {return getTypeString<T>();};
};
template <typename T>
class FilterFactory : public Loki::SingletonHolder < Loki::Factory< filters::FilterBase<T>, std::string >,
Loki::CreateUsingNew,
Loki::LongevityLifetime::DieAsSmallObjectChild >
{
//empty
};
Factory Registration Macro
#define ROS_REGISTER_FILTER(c,t) \
filters::FilterBase<t> * Filters_New_##c##__##t() {return new c<t>;}; \
bool ROS_FILTER_## c ## _ ## t = \
filters::FilterFactory<t>::Instance().Register(#c "<" #t ">", Filters_New_##c##__##t);
Example Filter
/** \brief A median filter which works on double arrays.
*
*/
template <typename T>
class MeanFilter: public FilterBase <T>
{
public:
/** \brief Construct the filter with the expected width and height */
MeanFilter();
/** \brief Destructor to clean up
*/
~MeanFilter();
virtual bool configure(unsigned int elements_per_observation, const std::string& number_of_observations);
/** \brief Update the filter and return the data seperately
* \param data_in T array with length elements_per_observation
* \param data_out T array with length elements_per_observation
*/
virtual bool update(T const * const data_in, T* data_out);
protected:
T * data_storage_; ///< Storage for data between updates
uint32_t last_updated_row_; ///< The last row to have been updated by the filter
uint32_t iterations_; ///< Number of iterations up to number of observations
uint32_t number_of_observations_; ///< Number of observations over which to filter
uint32_t elements_per_observation_; ///< Number of elements per observation
bool configured_;
};
ROS_REGISTER_FILTER(MeanFilter, double)
ROS_REGISTER_FILTER(MeanFilter, float)
Example Factory Usage
filters::FilterBase<float> * a_filter = filters::FilterFactory<float>::Instance().CreateObject("TestFilter<float>"); filters::FilterBase<float> * a1_filter = filters::FilterFactory<float>::Instance().CreateObject("TestFilter<float>"); filters::FilterBase<double> * b_filter = filters::FilterFactory<double>::Instance().CreateObject("TestFilter<double>"); printf("a is of type: %s\n b is of type: %s \n", a_filter->getType().c_str(), b_filter->getType().c_str()); a_filter->configure(4, ""); delete a_filter; delete a1_filter; delete b_filter;
FIlter Chain Object
template <typename T>
class FilterChain
{
public:
/** \brief Create the filter chain object */
FilterChain()
/** \brief Configure the filter chain
* This will call configure on all filters which have been added
* as well as allocate the buffers*/
bool configure(unsigned int size);
/** \brief Add filters to the list of filters to run on incoming data
* This will not configure, you must call configure before they will
* be useful. */
bool add(const std::string& filter_name, const std::string& filter_arguments);
/** \brief Clear all filters from this chain */
bool clear();
/** \brief process data through each of the filters added sequentially */
bool update(const T* const data_in, T* data_out);
~FilterChain();
Meeting agenda
Conclusion
Package status change mark change manifest)
![/!\](./rostheme/img/alert.png "/!\")
Action items that need to be taken. 
Major issues that need to be resolved 
find a way to use the template type of the factory instead of passing the type in a string. - create/use a way to delete the factory generated filters
- document that all filters should be realtime safe
- except for configure
- want to see intermediate values for debugging
- provide facility to get callback for intermediate values
- callback will provide name of filter and resultant values pointer
- blocking on filter progress
- standardize arguments with something (yaml, tinyxml, key value pair?, ros::node::param syntax)
- standard way to do vectors with examples
- initialize/add from xml would be useful
- simpler interface to tinyxml which might be extended to controllers if it works well
Stu:
- I would feel better if update took std::vector's instead of plain arrays so that we avoid memory issues.
- Is the getType method really necessary?
FilterChain::add should not create and configure the filter itself. Instead, it should take ownership of a filter constructed elsewhere
FilterChain::add should check that the order of the filter matches the order of the chain (order = number of data elements. Maybe "width" is a better word)