Skip to content

Fix load hex file - #175

Merged
jreineckearm merged 4 commits into
eclipse-cdt-cloud:mainfrom
StavrosKolliasArm:fix-load-hex
Sep 9, 2026
Merged

jreineckearm merged 4 commits into
eclipse-cdt-cloud:mainfrom
StavrosKolliasArm:fix-load-hex

Conversation

@StavrosKolliasArm

Copy link
Copy Markdown
Contributor

What it does

Fixes the issue
#172

Removes dependancy nrf-intel-hex

How to test

Review checklist

Reminder for reviewers

@jreineckearm jreineckearm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Apologies for the long delay in the review.
Implementation looks good. See a few questions/suggestions.

How much of the standard does it support? IIRC, other packages we looked at in the past usually missed one or the other part.

Still need to take it for a spin.

Comment thread src/plugin/memory-storage.ts Outdated
Comment thread src/plugin/memory-storage.ts
Comment thread src/common/intel-hex.ts
Comment thread src/common/intel-hex.ts
Comment thread src/common/intel-hex.spec.ts
@StavrosKolliasArm
StavrosKolliasArm marked this pull request as ready for review September 8, 2026 15:28
@jreineckearm jreineckearm moved this from Paused to In Review in CDT Cloud Debug (Experimental) Sep 8, 2026

@jreineckearm jreineckearm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code changes look good. Taking for a spin in the morning before merge.

@jreineckearm jreineckearm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the updates, @StavrosKolliasArm .

I took it for another spin and compared behavior with last Memory Inspector release (1.2.0):

  • Only difference between files written with old and new writer is that the new writer adds an additional line ending at the end of file. I'd expect this to do no harm and verified it can be loaded with the old reader.
  • I was able to read back now a file stored for a memory location > 0x80000000 (Arm v7-M based target, i.e. 32-bit address space). The same file failed with the old reader.
  • I tried a couple of odd lengths, which look good.

Happy to merge, thanks a lot for your contribution!

@jreineckearm
jreineckearm merged commit 182afb0 into eclipse-cdt-cloud:main Sep 9, 2026
5 checks passed
@github-project-automation github-project-automation Bot moved this from In Review to To Validate in CDT Cloud Debug (Experimental) Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants