|
Bugzilla – Full Text Bug Listing |
| Summary: | cannot use config paths to get a handle on an object | ||
|---|---|---|---|
| Product: | ns-3 | Reporter: | Mathieu Lacage <mathieu.lacage> |
| Component: | core | Assignee: | Mathieu Lacage <mathieu.lacage> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | gjcarneiro, ns-bugs, raj.b |
| Priority: | P3 | ||
| Version: | pre-release | ||
| Hardware: | All | ||
| OS: | All | ||
| Bug Depends on: | |||
| Bug Blocks: | 280 | ||
| Attachments: |
proposed patch
update |
||
This seems like an intuitive feature to offer (also for being able to check the existence of the device), so I would support adding it. While thinking about this some more tonight, it occured to me that we could use this to solve partly our error-reporting problems in the Config API. What about something like this:
class MatchContainer
{
public:
Ptr<Object> Get (uint32_t i) const;
uint32_t GetN (void) const;
std::string GetPath (uint32_t i) const;
void Connect (std::string traceSource, const CallbackBase &);
void Set (std::string attribute, const AttributeValue &v);
private:
std::vector<std::pair<Ptr<Object>,std::string> > m_matches;
};
namespace Config {
MatchContainer LookupMatches (std::string path);
}
(In reply to comment #0) > some users asked me today why it was not possible to use > /NodeList/x/DeviceList/y to get access to a specific device. > > Of course, the answer is that there is not necessarily a single match for a > specific string so, we would have to provide this: > > namespace Config { > > std::vector<Ptr<Object> > LookupMatching (std::string path); > > } > It seems to me that there are four types of things the config strings can specify: trace sources, nodes, devices, and things aggregated to a node (I feel like I'm forgetting some other use of these though...). In the specific cases of nodes or devices, it might be nice to have different APIs that return a more specific container: namespace Config { NodeContainer LookupMatchingNodes (std::string path); NetDeviceContainer LookupMatchingNetDevices (std::string path); std::vector<Ptr<Object> > LookupMatchingObjects (std::string path); } And I can't think of why anyone would want to directly get a handle to a TracedValue or TracedCallback, so perhaps this should be disallowed? (In reply to comment #3) > It seems to me that there are four types of things the config strings can > specify: trace sources, nodes, devices, and things aggregated to a node (I feel > like I'm forgetting some other use of these though...). In the specific cases yes. There are many more objects which can be accessed through the config API. Basically, anything which derives from Object. > of nodes or devices, it might be nice to have different APIs that return a more > specific container: > > namespace Config { > > NodeContainer LookupMatchingNodes (std::string path); > NetDeviceContainer LookupMatchingNetDevices (std::string path); > std::vector<Ptr<Object> > LookupMatchingObjects (std::string path); > > } > > And I can't think of why anyone would want to directly get a handle to a > TracedValue or TracedCallback, so perhaps this should be disallowed? There is no need to disallow it: it is simply impossible because these are not subclasses of the Object base class so, they cannot be returned in an array of Ptr<Object>. (In reply to comment #2) > While thinking about this some more tonight, it occured to me that we could use > this to solve partly our error-reporting problems in the Config API. What about > something like this: > > class MatchContainer > { > public: > Ptr<Object> Get (uint32_t i) const; > uint32_t GetN (void) const; > std::string GetPath (uint32_t i) const; > > void Connect (std::string traceSource, const CallbackBase &); > void Set (std::string attribute, const AttributeValue &v); > private: > std::vector<std::pair<Ptr<Object>,std::string> > m_matches; > }; > > namespace Config { > > MatchContainer LookupMatches (std::string path); > > } > again, this looks like a good idea to me Created attachment 271 [details]
proposed patch
Created attachment 272 [details]
update
*** Bug 164 has been marked as a duplicate of this bug. *** I hate the name MatchContainer. It has negative information value (confuses the reader more than clarifies, it sounds like more an object to hold the results of a regular expression match operation). I think something like NamedObjectList is more clear. But I still think supporting names in NodeList is by far the option that makes most sense IMHO. (In reply to comment #9) > I hate the name MatchContainer. It has negative information value (confuses > the reader more than clarifies, it sounds like more an object to hold the > results of a regular expression match operation). Which is exactly that. > > I think something like NamedObjectList is more clear. I think that you are confused. This is really a regexp match and we return the result of the match. > > But I still think supporting names in NodeList is by far the option that makes > most sense IMHO. That is a different issue, unrelated to this bug. (In reply to comment #10) > I think that you are confused. This is really a regexp match and we return the > result of the match. Righ, I was confused! That's what you get for reading bug reports in a hurry... :P So never mind my previous comments. So, looking at this more closely, I like it very much. Only two comments: 1. > void Set (std::string attribute, const AttributeValue &v); I would prefer SetAttribute here, or maybe SetAttributeForeach. It took me a few seconds to figure out what this 'Set' was supposed to do, until I figured out it must be "set an attribute on the object(s) pointed to by the trace path. Also it makes it more consistent with ObjectBase::SetAttribute; 2. This is a great opportunity to factor in a fix for bug #127 ;-) i.e. provide a way to obtain the callback signature for a trace path. So, if you could add a method like const std::type_info & GetCallbackType (), or GetCallbackType (int i), it should be enough to get tracing working for Python bindings, with some additional effort on my part. (In reply to comment #11) > So, looking at this more closely, I like it very much. Only two comments: > > 1. > void Set (std::string attribute, const AttributeValue &v); > I would prefer SetAttribute here, or maybe SetAttributeForeach. It took me > a few seconds to figure out what this 'Set' was supposed to do, until I figured > out it must be "set an attribute on the object(s) pointed to by the trace path. > Also it makes it more consistent with ObjectBase::SetAttribute; I agree but I tried to be consistent with Config::Set :/ > 2. This is a great opportunity to factor in a fix for bug #127 ;-) i.e. > provide a way to obtain the callback signature for a trace path. So, if you > could add a method like const std::type_info & GetCallbackType (), or > GetCallbackType (int i), it should be enough to get tracing working for Python > bindings, with some additional effort on my part. how about you implement it in a branch, and, then, I will review the changes you needed to make to the callback code ? (In reply to comment #12) > (In reply to comment #11) > > 2. This is a great opportunity to factor in a fix for bug #127 ;-) i.e. > > provide a way to obtain the callback signature for a trace path. So, if you > > could add a method like const std::type_info & GetCallbackType (), or > > GetCallbackType (int i), it should be enough to get tracing working for Python > > bindings, with some additional effort on my part. > > how about you implement it in a branch, and, then, I will review the changes > you needed to make to the callback code ? > OK, but I probably won't get to it any time soon. I remember I had a working patch once, but then NS 3 tracing was completely refactored and the patch is now useless. But I remember how hard it was to produce the patch, as it has very complicated C++ code in there. I don't really need tracing in python for my PhD work, so this is very low priority for me... (In reply to comment #7) > Created an attachment (id=272) [details] > update > +1 changeset 985324e2caaa |
some users asked me today why it was not possible to use /NodeList/x/DeviceList/y to get access to a specific device. Of course, the answer is that there is not necessarily a single match for a specific string so, we would have to provide this: namespace Config { std::vector<Ptr<Object> > LookupMatching (std::string path); }