Skip to content

Conversation

@dirwin5
Copy link

@dirwin5 dirwin5 commented May 30, 2022

rtc_string in ICM v9.5.5 has carriage returns rather than newline characters. Added line to replace these.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@dfmore
Copy link
Contributor

dfmore commented Jun 24, 2022

Hi @dirwin5, thank you for this pull request. Could you give me an example of when the old syntax could be problematic in 9.5.5? I've just tried it in the new version of ICM and it seems to work as expected. Thanks

@sancarn
Copy link
Contributor

sancarn commented Jun 24, 2022

Personally I would be against this PR. \r\n is typical for Windows OS, and editors such as notepad often require \r\n as a line ending. So personally I think ICM, as a Windows-first platform, and thus this script also should keep the \r\n line endings.

@dirwin5
Copy link
Author

dirwin5 commented Jun 27, 2022

Hi @dfmore, I've attached an image showing the file exported using the old syntax (left) vs rtc data manually exported from the ICM UI (right) using ICM v9.5.5. There are a lot of blank lines with the old syntax export due to the \r\n line endings.

Using the old syntax with newer versions of ICM, the export seems to follow the format on the right ok, as the exported data seems to have \n line endings rather than \r\n.

The screenshot shows the exported files opened using notepad on Windows 10.

Capture

@sancarn
Copy link
Contributor

sancarn commented Jun 27, 2022

Mhm.... it could be that ruby replaces all \r and \n with \r\n thus resulting in \r\n ==> \r\n\r\n...

Looking up online it seems there is a line ending character $\ and it seems this is could be the reason why file.write isn't working. Supposedly using file.puts doesn't have this issue.

@dirwin5 can you check File.open(path + exported_file, 'w') { |file| file.puts(rtc_string) } instead of your check. Otherwise could you upload the 2 files to a GIST so we can inspect the line endings properly?

@dirwin5
Copy link
Author

dirwin5 commented Jul 6, 2022

Hi @sancarn and @dfmore, sorry for the delayed response, I've been away for a few days.

I tried replacing file.write with file.puts, however I found that this does not change the output file. There are still extra blank lines in the v9.5.5 export.

I tested adding p rtc_string after File.open(path + exported_file, 'w') { |file| file.puts(rtc_string) } to see the printed line endings clearly.

  • In ICM version 9.5.5 the line this prints starts "!Version=1,type=RTC,charset=UTF8\r\n

  • whereas in ICM version 2021.5.0 the printed line starts "!Version=1,type=RTC,encoding=UTF8 \n

I believe ICM version 9.5.5 (and earlier) uses \r\n line endings, and this was changed in later versions to \n. My pull request would replace \r\n line endings with \n, which should fix the formatting of v9.5.5 exports, without affecting formatting of exports from later versions.

I have attached output files from both v9.5.5 and v2021.5.0 if you want to inspect the line endings. Note that these two exports are for different model networks so their content is different. Their formatting and line endings are visible however.

exported_rtc_v9.5.5.txt
exported_rtc_v2021.5.0.txt

@sancarn
Copy link
Contributor

sancarn commented Jul 6, 2022

  • In ICM version 9.5.5 the line this prints starts "!Version=1,type=RTC,charset=UTF8\r\n
  • whereas in ICM version 2021.5.0 the printed line starts "!Version=1,type=RTC,encoding=UTF8 \n

@dirwin5 So are you saying ICM version 2021.5.0 doesn't allow the import of files with \r\n line endings?

If so I'd say the bug here is really in ICM version 2021.5.0. The format for these files should remain static and the parser should remain backwards compatible. I feel the bug needs to be fixed in ICM itself and/or at least the code made to allow for either \r\n or \n.

@dfmore If you can find out what version this changed on, then we can change the script to do \r\n or \n dependent on ICM version. I feel that would be better, as otherwise currently the script will work for some people and not work for others...


EDIT:

I think I misunderstood again... You can't import/export RTC correct? And I'd imagine if you do row_object['rtc_data']=str then in ICM version 2021 it would require \r\n but in previous versions it would require \n. So really this has to do with an import target on the script... And/or changing the import script to handle this better. I believe replacing \r\n with \n isn't really the answer here.

@dfmore If you could get the version that this change occurred that'd be great, then we can essentially add:

if semver(Application.version) > "xxxx.xx.xxxx" 
  str.gsub(/\r?\n/,"\n")
end

to the import script.

@dirwin5
Copy link
Author

dirwin5 commented Jul 6, 2022

Hi @sancarn, this only seems to affect the exported rtc file when using the ruby script. Import seems to be fine with the original code.

The main reason I came across this issue is because I was using the ruby script to export rtc data from a model in ICM v9.5.5, and then using that rtc data in a separate external tool. There was a difference in file format which caused issues trying to read the data.

  • In ICM v9.5.5, using the ruby script to export rtc data gives the following file:
    image

  • Whereas exporting RTC data from the ICM UI in v9.5.5 (Network > Export > RTC data...) gives the following file:
    image

  • In ICM v2021.5, the exported file format is the same using the ruby script or exporting RTC data from the UI:
    image

@sancarn
Copy link
Contributor

sancarn commented Jul 6, 2022

@dirwin5 I see! Those images really explain the story much better! So essentially Ruby pre 2021 (or pre some version) errornously exported RTC data with \r\n when actually \n is needed when compared to Export > RTC data... spec. ICM post v2021.5 (or post someversion) has since fixed this issue, but this examples library should remain as agnostic as possible version wise. So the suggested change is to fix \r\n==>\n if it exists.

Whereas exporting RTC data from the ICM UI in v9.5.5 (Network > Export > RTC data...) gives the following file:

My concern is, if you try to import exported_rtc_v9.5.5_manual.rtc into ICM 9.5.5 with ruby, I.E:

network = WSApplication.current_network
row_object = network.row_object('hw_rtc',nil)  
rtcdata = File.read("path/to/exported_rtc_v9.5.5_manual.rtc")
network.transaction_begin
  row_object['rtc_data'] = rtcdata
  row_object.write
network.transaction_commit

Does this work as expected? Because if row_object['rtc_data'] is exporting with \r\n, i'd be concerned that row_object['rtc_data']= also requires \r\n line endings...

Alternatively you could test:

# Test me in ruby 9.5.5
network = WSApplication.current_network
row_object = network.row_object('hw_rtc',nil)  
network.transaction_begin
  row_object['rtc_data'] = row_object['rtc_data'].gsub(/\r\n/,"\n")
  row_object.write
network.transaction_commit
p row_object['rtc_data']

My concern/expectation is this might (if it works) accidentally keep \r characters resulting in \r\r\n... Or it will simply error.

Would you be able to test in 9.5.5? and perhaps include the result of the 2nd code (when ran in 9.5.5) in your response.


If \r\n is required for certain ICM versions then the ideal solution would be a larger change:

# Example 1: Export RTC from current network to a text file
if mode == 1
  #FIX: Ensure fix line endings to standard ICM spec. Force \r\n line endings to \n. See PR #8.
  File.write(path + exported_file, row_object['rtc_data'].gsub(/\r?\n/,"\n"))
  puts "File \'#{exported_file}\' exported to path \'#{path}\'"
end

# Example 2: Import RTC from a file into current network
if mode == 2
  imported_rtc = File.read(path + imported_file)
  network.transaction_begin
    #FIX: Version check before import and ensure line endings match spec for ruby at the time. See PR #8.
    row_object['rtc_data'] = import_rtc.gsub(/\r?\n/, WSApplication.version <= SOMEVERSION ? "\r\n" : "\n")
    row_object.write
  network.transaction_commit
  puts "RTC imported from file \'#{imported_file}\' on path \'#{path}\'"
end

(The above example is mostly unchanged from yours with a small change to mode 2)


Alternatively might be able to fix with begin/rescue depending on whether there are errors?

    #FIX: Version check before import and ensure line endings match spec for ruby at the time. See PR #8.
    begin
      row_object['rtc_data'] = import_rtc
    rescue # Attempt with \r\n line endings instead of \n
      row_object['rtc_data'] = import_rtc.gsub(/\r?\n/, "\r\n")
    end

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