POETSII / Orchestrator

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

Duplicate device instances are ignored #325

Open m8pple opened 2 years ago

m8pple commented 2 years ago

If a graph contains devices with duplicate device instances, then one of them is silently ignored rather than raising an error.

m8pple commented 2 years ago

See PR #326 for test-case.

mvousden commented 2 years ago

If a graph contains devices with duplicate device instances, then one of them is silently ignored rather than raising an error.

Yeah, that's not supposed to happen.

@DrongoTheDog thoughts?

mvousden commented 2 years ago

On Sun, Jul 17, 2022 at 12:15:57PM +0100, Andrew Brown wrote:

You know, I have even have a vague recollection of this, but I thought "Screw it, what's the harm in ignoring it?".

Also - philosophical point - the natuiral place to get this is in the Validator, but it's not really a validation issue - you can't capture name uniqueness in the validation grammar.

I think:

  1. It could/should be ignored; it have never have any adverse effects (other than it indictates the monkey has cocked up).

  2. It should therefore be posted as a Warning.

  3. The natural place to do this is by the conditional that traps it, but I don't have the time right now to hunt it.

  4. OK, you knew I would:

DevI_t * DS_XML::_DevI_t( .... )

My source, line 150:

if (pD->Def()!=0) { // Already defined? fprintf(fd,"(%3u,-) W%3u: Device %s already defined on line %u\n", pn->lin,++wcnt,dname.c_str(),pD->Def()); // Cockup. Bail.

So it is reported, in the microlog for the datastructure build, as a build warning.

If you Post a warning to the LogServer, that's a bit more "in yer face", but on the other hand if you do that you'll have to Post every other microlog message for consistency, and that's exactly what the microlog is there to avoid.

ADB

mvousden commented 2 years ago

Agree, up until:

If you do [Post a warning to the LogServer] you'll have to Post every other microlog message for consistency, and that's exactly what the microlog is there to avoid.

We should really be posting that there are warnings, and point the user to the microlog if so. Let the operator walk to their own destruction if they wish, but at least we told them so.

Nobody reads the micrologs unless prompted, anyway (by design).

mvousden commented 2 years ago

On Mon, Jul 25, 2022 at 11:01:22AM +0100, ADB wrote:

On 18/07/2022 10:01, Mark Vousden wrote:

Agree, up until:

If you do [Post a warning to the LogServer] you'll have to Post every other microlog message for consistency, and that's exactly what the microlog is there to avoid. We should really be posting that there are warnings, and point the user to the microlog if so. Let the operator walk to their own destruction if they wish, but at least we told them so. Fair point Nobody reads the micrologs unless prompted, anyway (by design). I do?

Anyway, by all means create a "microlog not empty" flag for each command and Post a warning at the end if it's set.........

ADB