Feature create devices after bus creation#954
Conversation
There was a problem hiding this comment.
I'm not familiar with the changes that require creating the devices after the bus. I would expect the devices not to know anything about/depend on the type of bus, and currently the device factory creates them without any information on a bus. Can you please explain?
Maybe this is related to the fact that the device factory knows the controller manager, and this one knows the bus?
The factory currently needs the controller manager only for creating the host services device. I would like to get rid of this dependency, but have not yet had a closer look at alternatives.
| string value; | ||
| }; | ||
| typedef std::vector<optarg_struct> optarg_queue_type; | ||
|
|
There was a problem hiding this comment.
Please use a standard multimap instead of custom containers here, e.g.:
unordered_multimap<int, string> optargs;
There was a problem hiding this comment.
I'm wondering if this makes more sense as a FIFO Queue. With the multimap, can we guarantee that the arguments will come out in the same order they went in?
There was a problem hiding this comment.
@uweseimet I changed it to a double ended queue (deque). let me know if you have any objections!
There was a problem hiding this comment.
@akuker Ordering would indeed be an issue with the multimap, so we cannot use it here. I am going to approve the PR, but before you merge, please replace "typedef" by "using", as recommended for modern C++. A minor change only.
With the current implementation, the With the current implementation of GPIOBUS, this is fine. However, with the upcoming revision, we need to know what type of board we're connected to (fullspec/aibom/standard/gamernium) before we call the GPIOBUS::Init() function. See: Note that this change is NOT NEEDED currently. But, I'd prefer to do any small, benign changes early. If you prefer I can lump this in with the |
|
Also note that this moves the log level setting earlier in the start-up process. This can be useful when trying to debug issues during initialization. |
|
@akuker Looks as if my guess about the manager and its bus argument was quite right. Actually, I don't have a strong opinion on when to merge these changes. I'm fine with doing this early. I intend to resolve the dependency between the controller manager and the bus sooner or later, but this requires also to change the host services and I do not yet know how exactly this change should look like. Otherwise I would already have done it ;-). |
…ithub.com/akuker/RASCSI into feature_create_devices_after_bus_creation
|
Kudos, SonarCloud Quality Gate passed!
|
commit c41373d Author: Uwe Seimet <48174652+uweseimet@users.noreply.github.com> Date: Wed Nov 2 15:36:19 2022 +0100 Use lambdas for dispatcher, code cleanup, test updates (#958) * Using lambdas instead of member function pointers simplifies the command dispatching and reduces the code volume * Removed duplicate error handling * Fix for issue #956 * Unit test updates * Resolved SonarQube issues commit 31dd063 Author: akuker <34318535+akuker@users.noreply.github.com> Date: Wed Nov 2 08:58:59 2022 -0500 Create devices after bus creation (#954)
In preparation of the upcoming banana pi support, I needed to restructure how the program arguments are parsed. Currently, the 'bus' object needs to be created before we can parse the arguments. However, with the bpi updates, I need to have the program arguments as an input to creating the 'bus' object.
Attached is my proposed approach. Basically, it saves off the program args related to device creation, then processes them later (after the bus and associated controllers/executor/etc are created).