Ticket #508 (closed task: fixed)

Opened 11 years ago

Last modified 7 years ago

Convert ctrl_events to use new PropertySets and Exceptions

Reported by: srp Owned by: srp
Priority: normal Milestone:
Component: ctrl_events Keywords:
Cc: Blocked By:
Blocking: Project: LSST
Version Number:
How to repeat:

not applicable

Description (last modified by srp) (diff)

The events tree needs to be updated to use the new changes made to exceptions and PropertySets?

Change History

comment:1 Changed 11 years ago by srp

  • Status changed from new to assigned
  • Description modified (diff)

comment:2 Changed 11 years ago by srp

  • Status changed from assigned to inTicketWork
  • Type changed from defect to task

comment:3 Changed 11 years ago by srp

As of svn6807:

PropertySets? are now being used by events, and people can use this svn revision to get this.

Exceptions have also been integrated. Swig has a problem throwing exceptions in constructors that use Policy since Policy hasn't yet been updated yet. I expect this to go away when Policy is rev-ed.

All tests pass except tests/EventSystem_3.py and tests/EventSystem_7.py (because of Policy).

comment:4 Changed 11 years ago by srp

  • reviewstatus changed from notReady to needsReview

These changes have been merged back into the ctrl/events trunk. All tests pass, except EventSystem_3.py and EventSystem_7.py. This is because Policy hasn't been upgraded to use the new PropertySet/Exceptions? changes.

comment:5 Changed 11 years ago by srp

  • Milestone set to DC3a MW Event Framework

comment:6 Changed 11 years ago by srp

  • Owner changed from srp to RayPlante
  • Status changed from inTicketWork to inStandardsReview

comment:7 Changed 11 years ago by srp

Revision svn6987 committed to trunk with fixes for Policy. Examples directory updated for PropertySet?. Ready for review.

comment:8 Changed 10 years ago by RayPlante

  • Owner changed from RayPlante to srp

My review recommendations are all minor:

  • updated table file
  • since there is no Events class, Events.h should be a events.h; recommend putting this a include/lsst/ctrl/events.h
    ramifications:
    • update include in tests/Events_[12].cc
  • *.h: current practice is to reflect namespace in include guards (e.g. LSST_CTRL_EVENTS_EVENTTRANSMITTER_H)
  • src: current practice is to have all .cc files for primary namespace (in this case, lsst::ctrl::events, to be at the top level; I recommend moving contents of src/events to src.
    ramifications:
    • update lib/SConstruct
  • documentation: use \file to head the include files; annotate the class definition to describe the class. This will get rid of the superfluous class entries (that have nothing in them) in doxygen on the Classes page. Recommend adding additional explanation of class (particularly EventReceiver? and Event Transmitter) in the class annotation (in *.h); alternatively, class documentation could be put in the .cc file (within the namespace block) using \class.
    ramifications:
    • some messaging of documentation wording applied (see below).

I have implemented all of these recommendations in source:DMS/ctrl/events/branches/508-rev. You can either (a) merge all these into the trunk yourself if you accept them (perhaps after some modifications), (b) ask me to merge them, or (c) handle them each of the recommendations yourself in the trunk.

comment:9 Changed 10 years ago by srp

All recommended changes have been made and are checked into the trunk. One additional change was to move one method signature in EventTransmitter?.h from private to public to facilitate event typing, which we'll do at a future date. The code was already in place, I thought I'd make it public to help facilitate backwards compatibility with the original version of events that had typing as part of the API.

comment:10 Changed 10 years ago by srp

  • reviewstatus changed from needsReview to reviewed

Review and changes complete. Closing this.

comment:11 Changed 10 years ago by srp

  • reviewstatus changed from reviewed to selfReviewed

comment:12 Changed 10 years ago by srp

can't change status to closed....

comment:12 Changed 10 years ago by dgehrig

  • Status changed from inStandardsReview to inTrunkMerge

comment:13 Changed 10 years ago by srp

  • Status changed from inTrunkMerge to inQaReview

comment:14 Changed 10 years ago by srp

  • Status changed from inQaReview to closed
  • Resolution set to fixed

comment:15 Changed 7 years ago by robyn

  • Milestone DC3a MW Event Framework deleted

Milestone DC3a MW Event Framework deleted

Note: See TracTickets for help on using tickets.