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

M318 and M351 checks #238

Closed
wants to merge 1 commit into from

Conversation

kossnikita
Copy link
Contributor

Message Number Type Message Text
M318 WARNING 'LEVEL' 'NAME' <'TAG'> is equal to <name>
M351 WARNING Peripheral 'TYPE' 'NAME' is equal to Peripheral name.

Doesn't add if it's equal to name:
Peripheral:

  • displayName
  • description
  • alternatePeripheral
  • groupName
  • headerStructName
    Register:
  • displayName
  • description
  • alternateRegister

I can't say that I'm happy with this method. This may be unexpected for the user.
But this is a very simple way to remove displayName duplicate name in my case. Patching this manually seems terrible.

@kossnikita kossnikita requested a review from a team as a code owner October 26, 2023 11:52
@burrbull
Copy link
Member

I can't say that I'm happy with this method.

Silently remove tags is very bad idea.
And this is not a place to patch.

@kossnikita
Copy link
Contributor Author

Can we do this as an option in the configuration, which can be activated by an argument in svd-tools?

@burrbull
Copy link
Member

Are those displayNames present in initial SVD or produced by patches?

Can we do this as an option in the configuration, which can be activated by an argument in svd-tools?

svdtools patch or svdtools convert?

@kossnikita
Copy link
Contributor Author

kossnikita commented Oct 26, 2023

Are those displayNames present in initial SVD or produced by patches?

In the original SVD.

Good question. I think that this should only be in the patch, since this is a significant content change.

@burrbull
Copy link
Member

If we are talking about svdtools patch and initially present displayNames it should be formed as rule. Similar to _clear_fields: https://github.com/stm32-rs/stm32-rs/blob/ea6c0312dfc79e74bf3cbbcb735a3c32328dcfb2/devices/stm32g071.yaml#L6

If it is incorrectly produced by patch, it is a bug.

@burrbull
Copy link
Member

Actually I don't understand what sense of displayName and why it should be different from name.
Possibly it is something C-relared.

@kossnikita
Copy link
Contributor Author

Similar to _clear_fields

Another undocumented feature... 😅
I will take care about it someday.

I think displayName should be used in debugger viewer for some cases. And it should be different because it's equal to name by default. This makes my svds much smaller. I can't imagine any others reasons. It's only warning in any case.

@kossnikita
Copy link
Contributor Author

I'm not sure I understand you. Are you proposing to introduce a new feature for clearing displayName similar to _clear_fields? Or does such a feature already exist?

@burrbull
Copy link
Member

New feature. For already present displayNames.

More investigation for other cases.

@kossnikita kossnikita closed this Oct 27, 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