| Message ID | 20190928152238.23752-6-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On Sat, Sep 28, 2019 at 06:22:31PM +0300, Laurent Pinchart wrote: > The ControlList::update() method is unused. While it is meant to fulfil > a need of applications, having no user means that it is most probably > not correctly designed. Remove the method, we will add it back later if > needed. > This indeed removes the discussion if this operation should have been called update() or merge() If not used, let's drop it Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/controls.h | 2 -- > src/libcamera/controls.cpp | 28 ---------------------- > test/controls/control_list.cpp | 43 ---------------------------------- > 3 files changed, 73 deletions(-) > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > index 9698bd1dd383..d4a74ada1b6a 100644 > --- a/include/libcamera/controls.h > +++ b/include/libcamera/controls.h > @@ -158,8 +158,6 @@ public: > val->set<T>(value); > } > > - void update(const ControlList &list); > - > private: > const ControlValue *find(const ControlId &id) const; > ControlValue *find(const ControlId &id); > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp > index 8886660b7617..62cb2ff822bb 100644 > --- a/src/libcamera/controls.cpp > +++ b/src/libcamera/controls.cpp > @@ -511,32 +511,4 @@ ControlValue *ControlList::find(const ControlId &id) > return &controls_[&id]; > } > > -/** > - * \brief Update the list with a union of itself and \a other > - * \param other The other list > - * > - * Update the control list to include all values from the \a other list. > - * Elements in the list whose control IDs are contained in \a other are updated > - * with the value from \a other. Elements in the \a other list that have no > - * corresponding element in the list are added to the list with their value. > - * > - * The behaviour is undefined if the two lists refer to different Camera > - * instances. > - */ > -void ControlList::update(const ControlList &other) > -{ > - if (other.camera_ != camera_) { > - LOG(Controls, Error) > - << "Can't update ControlList from a different camera"; > - return; > - } > - > - for (auto it : other) { > - const ControlId *id = it.first; > - const ControlValue &value = it.second; > - > - controls_[id] = value; > - } > -} > - > } /* namespace libcamera */ > diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp > index 053696814b67..8469ecf09439 100644 > --- a/test/controls/control_list.cpp > +++ b/test/controls/control_list.cpp > @@ -141,49 +141,6 @@ protected: > return TestFail; > } > > - /* > - * Test list merging. Create a new list, add two controls with > - * one overlapping the existing list, merge the lists and clear > - * the old list. Verify that the new list is empty and that the > - * new list contains the expected items and values. > - */ > - ControlList newList(camera_.get()); > - > - newList.set(controls::Brightness, 128); > - newList.set(controls::Saturation, 255); > - newList.update(list); > - > - list.clear(); > - > - if (list.size() != 0) { > - cout << "Old List should contain zero items" << endl; > - return TestFail; > - } > - > - if (!list.empty()) { > - cout << "Old List should be empty" << endl; > - return TestFail; > - } > - > - if (newList.size() != 3) { > - cout << "New list has incorrect size" << endl; > - return TestFail; > - } > - > - if (!newList.contains(controls::Brightness) || > - !newList.contains(controls::Contrast) || > - !newList.contains(controls::Saturation)) { > - cout << "New list contains incorrect items" << endl; > - return TestFail; > - } > - > - if (newList.get(controls::Brightness) != 10 || > - newList.get(controls::Contrast) != 20 || > - newList.get(controls::Saturation) != 255) { > - cout << "New list contains incorrect values" << endl; > - return TestFail; > - } > - > return TestPass; > } > > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 9698bd1dd383..d4a74ada1b6a 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -158,8 +158,6 @@ public: val->set<T>(value); } - void update(const ControlList &list); - private: const ControlValue *find(const ControlId &id) const; ControlValue *find(const ControlId &id); diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 8886660b7617..62cb2ff822bb 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -511,32 +511,4 @@ ControlValue *ControlList::find(const ControlId &id) return &controls_[&id]; } -/** - * \brief Update the list with a union of itself and \a other - * \param other The other list - * - * Update the control list to include all values from the \a other list. - * Elements in the list whose control IDs are contained in \a other are updated - * with the value from \a other. Elements in the \a other list that have no - * corresponding element in the list are added to the list with their value. - * - * The behaviour is undefined if the two lists refer to different Camera - * instances. - */ -void ControlList::update(const ControlList &other) -{ - if (other.camera_ != camera_) { - LOG(Controls, Error) - << "Can't update ControlList from a different camera"; - return; - } - - for (auto it : other) { - const ControlId *id = it.first; - const ControlValue &value = it.second; - - controls_[id] = value; - } -} - } /* namespace libcamera */ diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp index 053696814b67..8469ecf09439 100644 --- a/test/controls/control_list.cpp +++ b/test/controls/control_list.cpp @@ -141,49 +141,6 @@ protected: return TestFail; } - /* - * Test list merging. Create a new list, add two controls with - * one overlapping the existing list, merge the lists and clear - * the old list. Verify that the new list is empty and that the - * new list contains the expected items and values. - */ - ControlList newList(camera_.get()); - - newList.set(controls::Brightness, 128); - newList.set(controls::Saturation, 255); - newList.update(list); - - list.clear(); - - if (list.size() != 0) { - cout << "Old List should contain zero items" << endl; - return TestFail; - } - - if (!list.empty()) { - cout << "Old List should be empty" << endl; - return TestFail; - } - - if (newList.size() != 3) { - cout << "New list has incorrect size" << endl; - return TestFail; - } - - if (!newList.contains(controls::Brightness) || - !newList.contains(controls::Contrast) || - !newList.contains(controls::Saturation)) { - cout << "New list contains incorrect items" << endl; - return TestFail; - } - - if (newList.get(controls::Brightness) != 10 || - newList.get(controls::Contrast) != 20 || - newList.get(controls::Saturation) != 255) { - cout << "New list contains incorrect values" << endl; - return TestFail; - } - return TestPass; }
The ControlList::update() method is unused. While it is meant to fulfil a need of applications, having no user means that it is most probably not correctly designed. Remove the method, we will add it back later if needed. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- include/libcamera/controls.h | 2 -- src/libcamera/controls.cpp | 28 ---------------------- test/controls/control_list.cpp | 43 ---------------------------------- 3 files changed, 73 deletions(-)