POETSII / Orchestrator

The Orchestrator is the configuration and run-time management system for POETS platforms.
1 stars 1 forks source link

Nameserver #103

Open AlexRast opened 4 years ago

AlexRast commented 4 years ago

This adds the NameServer process and makes the Mothership inherit from SBase. Now that Mothership officially inherits from its expected base classes, its class files are given the 'proper' Mothership.h and Mothership.cpp names. I think Ns_el.h and Ns_el.cpp are probably not needed but I have not changed them and so far everything seems to work - tested with some small working examples using both default and application supervisors from heat plate and clock tree, and with some larger (multi-board) examples as well. There is a MothershipDummy class to aid in testing of the NameServer without the full mothership; it can probably eventually be dropped but might still come in handy when testing UserIO in future. I have added a few output messages when loading cores just to give the operator some sense of progress - note how SLOW these are to remind everyone of how valuable a thread safe multi-core/multi-board loader would be! I have manually merged non-conflicting updates to other files from the development branch so although it may show as being any number of commits behind (reflecting the point when the branch was created!) in fact most of the actual files should be up-to-date; of course with significant changes to nameserver, mothership and several related files including the PMsg_p and CMsg_p classes. For now the group of name services that can inject packets into a Tinsel core directly based upon a device name is not implemented, pending implementation of UserIO. But most of the rest of the name services commands are implemented and tested.

mvousden commented 4 years ago

Can we merge development into nameserver, so that the review only displays the changes you've made (as opposed all the changes that have been made since ca8bbaf)? i.e.:

git pull
git checkout nameserver
git merge development
... # resolve merge conflicts, potentially with some help
... # resolve changes introduced from renaming TMoth.{h,cpp} -> Mothership.{h,cpp}, potentially with some help
git commit
git push

If you're not confortable to do this, we can do this together, or explore other avenues. As it stands, it's difficult for me (as a reviewer) to see only the changes that you have introduced.

I'm around this afternoon (from about 1530).

heliosfa commented 4 years ago

Just to echo what Mark said, this need to have Development merged in properly (rather than manually) before it can be merged back into development.

It is incredibly bad practice to review and/or merge when there are conflicting files - the merge should be a clean one.

AlexRast commented 4 years ago

As seen (3 commits later...) this should fix everything.

heliosfa commented 4 years ago

When I compile these changes, I get some warnings ( warnings.txt). Please address these (they all look largely trivial). 🍠

A lot of these warnings look like a reversion of the changes made in #88 to address #31 to suppress warnings (e.g. d9340ccb70679a70d3fa636eaf05a959b38f69fd, 1faa0a9deb9371c325ed73447f1f8baf83eb865b and others look to have been undone)

AlexRast commented 4 years ago

@heliosfa's last comment doesn't seem to have a reply option - but is this something that could have happened because of the merge process? I would not be surprised. I don't remember seeing warnings pre-merge.

AlexRast commented 4 years ago

Some of the warnings look to have been related to merging, others though when I looked through them would clearly have been there anyway. In any case, these are all fixed together with removal of tabs and the makefile stuff.

AlexRast commented 4 years ago

Can I get everyone's acknowledgement that Ns_el.h/Ns_el.cpp are not in use nor intended for some future use? I have kept them in just in case someone screamed 'NOOO, don't delete that, we need it for xxx/I was planning to use it for yyy'.

heliosfa commented 4 years ago

Can I get everyone's acknowledgement that Ns_el.h/Ns_el.cpp are not in use nor intended for some future use? I have kept them in just in case someone screamed 'NOOO, don't delete that, we need it for xxx/I was planning to use it for yyy'.

I believe that you are correct - they are ADB's original pre-implementation

heliosfa commented 4 years ago

@heliosfa's last comment doesn't seem to have a reply option - but is this something that could have happened because of the merge process? I would not be surprised. I don't remember seeing warnings pre-merge.

Depending how the merge was done, potentially.

One of the changes you have merged in is adding -Wall and -Wpedanticto the make file, which turn on more warnings. The head of development builds cleanly with these flags, so any warning have been introduced by your changes/the merge you have done.

mvousden commented 4 years ago

Can I get everyone's acknowledgement that Ns_el.h/Ns_el.cpp are not in use nor intended for some future use? I have kept them in just in case someone screamed 'NOOO, don't delete that, we need it for xxx/I was planning to use it for yyy'.

Agree, get rid of it (we can always pull it out of the history if it becomes relevant again for some presently unknown reason).

AlexRast commented 4 years ago

Yes, I think the -Wpedantic in particular was responsible for a lot of the additional warnings coming up.