search

linsong / sedona

The Sedona Framework for building smart, networked embedded devices easier.
https://linsong.github.io/sedona
Other
34 stars 29 forks source link

sys improvements #40

Open AndreySV opened 5 years ago

AndreySV commented 5 years ago

resume of PR "sys improvements" #36

AndreySV commented 5 years ago

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:

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 commented 5 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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.