Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Topic: CommandLine #220

Merged

Conversation

kkoppolu1
Copy link
Collaborator

@kkoppolu1 kkoppolu1 commented Feb 3, 2023

Add command line arguments for structs

@kkoppolu1
Copy link
Collaborator Author

Reviews in this chain:
#220 Topic: CommandLine

@kkoppolu1
Copy link
Collaborator Author

kkoppolu1 commented Feb 3, 2023

# head base diff date summary
0 445078fc 7d095715 diff Feb 3 23:06 PM 5 files changed, 128 insertions(+), 43 deletions(-)
1 dde46269 ee3c763f diff Feb 3 23:08 PM 1 file changed, 1 insertion(+)
2 d1ff29bc ee3c763f diff Feb 3 23:50 PM 3 files changed, 8 insertions(+), 7 deletions(-)
3 53f20a49 ee3c763f diff Feb 7 1:02 AM 1 file changed, 4 deletions(-)
4 3d9b1b2e ee3c763f diff Feb 8 0:37 AM 1 file changed, 1 insertion(+), 1 deletion(-)
5 d1f0c93f ee3c763f diff Feb 8 2:13 AM 1 file changed, 1 insertion(+), 1 deletion(-)

Add command line arguments for structs
@kkoppolu1 kkoppolu1 force-pushed the kkoppolu/revup/master/CommandLine branch 2 times, most recently from dde4626 to d1ff29b Compare February 3, 2023 23:50
vspec/loggingconfig.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@erikbosch erikbosch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. I like the refactoring to use logging instead of simple prints

Fix copyright statement
@kkoppolu1
Copy link
Collaborator Author

Incremental Change for struct support: Add command line arguments

Description

Add command-line arguments to the VSS parser for struct support.
Details of the proposal in COVESA/vehicle_signal_specification#511

Two new arguments are added:

  1. Input file containing the types
  2. Output file for the types in the requested format (JSON for example)

The following checks are added:

  1. Overlays cannot be specified when data type support arguments are provided
  2. Only JSON format can be specified when data type support arguments are provided
  3. When the feature is requested, the user is informed that it is experimental.
  4. After all checks have passed, a NotImplemented exception is thrown.

Miscellaneous

  • Run autopep8 on all touched files
  • Replace print statements with logging in vspec2x.py and vspec2json.py

Example output:

INFO     Output to json format
INFO     Known extended attributes: 
WARNING  From VSS 4.0 the default behavior for printing uuid will change. Consider using --uuid or --no-uuid.
WARNING  Use of default VSS unit file is deprecated, please specify the unit file you want to use with the -u argument!
INFO     Loading vspec from /home/kkoppolu/workplace/vehicle_signal_specification/spec/VehicleSignalSpecification.vspec...
INFO     Calling exporter...
INFO     Generating JSON output...
INFO     Serializing compact JSON...
INFO     All done.

Testing

  • Ran pytest
  • Ran JSON converter using the standard catalog.

Signed-off-by: Krishna Koppolu <krishnachaitanya.85@gmail.com>

"""
Initialize logging
"""
logging.basicConfig(level=logging.DEBUG, format='%(levelname)-8s %(message)s')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now I assume DEBUG is OK as default level. But do we want to keep it as default in the long run, or should rather INFO be the default level?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default is WARNING. Given that this is a tool that is not running in a performance sensitive environment, INFO would be a better default. In the future, we can make it configurable, by adding a verbose flag to the CLI.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, would be easy to add flag (or env variable?) to control it.

Copy link
Collaborator Author

@kkoppolu1 kkoppolu1 Feb 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. a command line arg is pretty common. Updated to INFO in this PR.

Krishna Koppolu added 2 commits February 8, 2023 00:36
Update default log level to INFO
Fix reference to format name
@erikbosch erikbosch merged commit acf2557 into COVESA:master Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants