search

byte-motion / RNL_RAPIDLibrary

A standard library of functionality for the RAPID programming language
MIT License
0 stars 1 forks source link

Update RnLibLog.sys #32

Closed RobotSigmund closed 1 year ago

RobotSigmund commented 3 years ago

Closes #21

Added comments. Added proper variable naming. Simplified functionality. Added errormessages, easier to debug during config.

RobotSigmund commented 3 years ago

Addressed most of the comments. However have not implemented functionality for:

ALIAS string logFile FUNC logFile logFile_New( string filePath )

Is this needed? The module supports multiple logfiles.

The only difference in functionality would be that we could add multiple archives for each new object? Is this what we want?

RobotSigmund commented 3 years ago

Merging is blocked by a change request(?). I can't see anything that has not been commented/fixed. @TheHarvard + @AGus-RN .

TheHarvard commented 3 years ago

Addressed most of the comments. However have not implemented functionality for:

ALIAS string logFile FUNC logFile logFile_New( string filePath )

Is this needed? The module supports multiple logfiles.

The only difference in functionality would be that we could add multiple archives for each new object? Is this what we want?

At the moment, I am working on implementing objects and inheritance and I would like log-files to eventually migrate into an inheritable object.

To do this, it must have an associated data type that represents it, and a NEW constructor method to create new ones.

We can implement logging as is now with the adjustments you have made, and re-integrate it once the inheritance system is proven in the Wiig project, or i could give you an overview of the system and we could rewrite it to comply now before pushing.

Toughts?

RobotSigmund commented 3 years ago

I would like to have available the current functionality. It is simple to use and will fit into rw-systems without any dependencies, it is very useful, for instance when debugging a system delivered by another integrator. Or as a supplement to any very basic program.

I can also see the benefits of integrating logging by events. I think we need a second version for this. It would be nice if both functionalites can be provided from 1 module, but that will probably trigger dependency issues if our RN-library is not installed.

RobotSigmund commented 3 years ago

Bumping this. It should be implementet asap, just so noone grabs the old version currently in master branch.

TheHarvard commented 3 years ago

I would like to have available the current functionality. It is simple to use and will fit into rw-systems without any dependencies, it is very useful, for instance when debugging a system delivered by another integrator. Or as a supplement to any very basic program.

I can also see the benefits of integrating logging by events. I think we need a second version for this. It would be nice if both functionalites can be provided from 1 module, but that will probably trigger dependency issues if our RN-library is not installed.

we could add the original implementation to the REPO for non-compliant modules, see #64

But i also think it should be high priority to implement this in the RNL framework.

RobotSigmund commented 3 years ago

I can see in the Tine project repo that an old version of this module has been used. Please review this PR. It should be added asap or added to another repo for standardised multi-purpose module collection.