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

sys improvements #40

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

sys improvements #40

wants to merge 8 commits into from

Conversation

AndreySV
Copy link
Collaborator

@AndreySV AndreySV commented May 6, 2019

resume of PR "sys improvements" #36

AndreySV and others added 6 commits May 7, 2019 00:02
It's hard to fit any useful encoding scheme or other information into
7 charaters. SOX protocol doesn't set any limitation on name length.

CPT, Niagara 3.8 show long names without a problem, but on changing
name it is truncated to 7 characters.
EasyIO tool and devices support 31 length for component's name.

Signed-off-by: Andrey Skvortsov <[email protected]>
…message

Backtrace example:
java.lang.IllegalStateException: Unknown field id: 61
       at sedona.dasp.DaspMsg.doDecode(DaspMsg.java:153)
       at sedona.dasp.DaspMsg.<init>(DaspMsg.java:92)
       at sedona.dasp.DaspMessage.<init>(DaspMessage.java:27)
       at sedona.dasp.DaspSocket.dispatch(DaspSocket.java:337)
       at sedona.dasp.DaspSocketInterface$Receiver.run(DaspSocketInterface.java:166)
since we enlarge component's name from 7 chars to 31 chars, we need a
way to tell programming tools to know what size is used in a sedona app
so that the tool can support it easily based on sedona app
@AndreySV
Copy link
Collaborator Author

AndreySV commented May 9, 2019

To continue our conversation about introducing changes to sox/dasp protocol.

IMHO,
d674097 sedona: DaspMsg: add support of compNameLenMax header
introduces more problem than solves.

I've tried several programming tools with following results:

  • Niagara Workbench doesn't connect when receives unknown header field
  • CPT (0.5) silently closes connection
  • SAE doesn't connect either.

If we'd merge it into master, that would make this fork unusable for everyone!

Now about support of long component names in programming tools:

  1. all programming tools read long component names without any modifications out of the box
  2. all of them cut component names on rename or adding new component to 7 characters
  3. long name support can be added to Workbench and possibly to SAE

To add full support for long component names into Niagara Workbench it's necessary update sedona.jar and sedonac.jar in modules directory. But Workbench doesn't load normal jar files. It's required to add /META-INF/module.xml file to jar file to make able Workbench to load these update jar files as modules. These files can be simply taken from old existing sedona[c].jar files.
I've added module.xml into jar build process and will submit that in other PR with some other changes.

With new modules it's possible to give components long names. If Workbench is connected to old sedona runtime with short component names and user tries rename component to some long name, then new name is automatically trimmed to short name by sox kit (SoxCommands.sedona:627) and for new components long names are trimmed as well (App.sedona). Workbench automatically fetches new trimmed names from sedona runtime to update wiresheet. User immediately sees that name is trimmed. So there is no problem with mixed usage of old/new runtime in Workbench.

Most likely the same would apply to SAE, because it internally uses sedona[c].jar. The only problem is that all jar files are integrated into executable file and I couldn't find easy way to update sedona[c].jar files.

I think other programming tools can have similar behavior and there will be no requirements to enhance protocol or make some other changes to sox/dasp/sys kits.

My conclusion is that it's already possible to use new sedona mixed with old sedona usage with existing tools without much changes and problems.

@linsong
Copy link
Owner

linsong commented May 10, 2019

To continue our conversation about introducing changes to sox/dasp protocol.

IMHO,
d674097 sedona: DaspMsg: add support of compNameLenMax header
introduces more problem than solves.

I've tried several programming tools with following results:

  • Niagara Workbench doesn't connect when receives unknown header field
  • CPT (0.5) silently closes connection
  • SAE doesn't connect either.

If we'd merge it into master, that would make this fork unusable for everyone!

Thanks for the comprehensive testing!

I will revert the header from Dasp message since these programming tools are broken on this, although the headers are designed to be extensible.

Now about support of long component names in programming tools:

  1. all programming tools read long component names without any modifications out of the box
  2. all of them cut component names on rename or adding new component to 7 characters
  3. long name support can be added to Workbench and possibly to SAE

To add full support for long component names into Niagara Workbench it's necessary update sedona.jar and sedonac.jar in modules directory. But Workbench doesn't load normal jar files. It's required to add /META-INF/module.xml file to jar file to make able Workbench to load these update jar files as modules. These files can be simply taken from old existing sedona[c].jar files.
I've added module.xml into jar build process and will submit that in other PR with some other changes.

With new modules it's possible to give components long names. If Workbench is connected to old sedona runtime with short component names and user tries rename component to some long name, then new name is automatically trimmed to short name by sox kit (SoxCommands.sedona:627) and for new components long names are trimmed as well (App.sedona). Workbench automatically fetches new trimmed names from sedona runtime to update wiresheet. User immediately sees that name is trimmed. So there is no problem with mixed usage of old/new runtime in Workbench.

There is one issue here: there is a basic assumption that component's name should be unique among its siblings. although SoxCommands.sedona and App.sedona can trim the name, but the trimmed name can not be guaranteed to be unique. one solution is we improve the name trimming logics to make the trimmed name unique.

Even we can do the above improvement, I think it is still valuable to pass the component name length limit to programming tools. If programming tools know the limit, they can validate user's input before sending it to sedona, that can produce better user experience.

Most likely the same would apply to SAE, because it internally uses sedona[c].jar. The only problem is that all jar files are integrated into executable file and I couldn't find easy way to update sedona[c].jar files.

I think other programming tools can have similar behavior and there will be no requirements to enhance protocol or make some other changes to sox/dasp/sys kits.

My conclusion is that it's already possible to use new sedona mixed with old sedona usage with existing tools without much changes and problems.

After thinking it over, I perfer to adding a new Sox command to pass these limit data(now only component name length, in future I think there will be more), because:

  1. a new Sox command will not break any existing programming tool
  2. if a programming tool want to support longer name, it just need to query by the new Sox command
  3. the new Sox command can be extended for future limit data

@divisuals
Copy link
Collaborator

@AndreySV - this has been pending for a while. What's the status of this pull request?

If you are planning to add any commits, please rebase this with master as well, so that it can be reviewed/ merged soon! Thanks

@AndreySV
Copy link
Collaborator Author

Since 'sys: increase Component's name from 7 to 31 character' seems to be controversial, I'd drop this commit and merge the first two. Maybe we can come back to this feature later.

@linsong
Copy link
Owner

linsong commented Apr 14, 2020

Since 'sys: increase Component's name from 7 to 31 character' seems to be controversial, I'd drop this commit and merge the first two. Maybe we can come back to this feature later.

To transfer the component name length limit to programming tool, the Name/Value pair in versionMore seems a good choice, it is read at the very beginning of a Sox session and should not crash current programming tools.

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.

3 participants