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

add support for efinix .bit/.hex files (input only) #51

Merged
merged 9 commits into from
Mar 20, 2023

Conversation

danselmi
Copy link
Contributor

@danselmi danselmi commented Mar 9, 2023

This patch adds support for reading bit-stream files from efinix.

Can you point me to where I have to add the documentation about this format? I will then provide a second patch or amend this one.

@sierrafoxtrot sierrafoxtrot self-requested a review March 10, 2023 08:35
@sierrafoxtrot sierrafoxtrot marked this pull request as draft March 10, 2023 08:36
@sierrafoxtrot sierrafoxtrot self-assigned this Mar 10, 2023
@sierrafoxtrot
Copy link
Owner

This patch adds support for reading bit-stream files from efinix.

This is great @danselmi and I'm looking forward to this being merged. Always love when we add a new format and you'd be bringing the number to 44 in total!

Can you point me to where I have to add the documentation about this format? I will then provide a second patch or amend this one.

The doco is written in good old (emphasis on old :-) ) nroff format. This is then used to generate man pages, reference manual and website at compile time.

Basically if you added doc/man5/srec_efinix.5 the cmake build system should just pick it up automagically. If you were looking for an example, the srec_vmem.5 would be similar and may be familiar if programmed logic is your thing! If you are looking for something complex with diagrams etc, doc/man5/srec_intel.5 and doc/man5/srec_mif.5 are great thorough examples.

Additionally, for contributions of file formats, we are going to need an addition to the regression test suite. These are in test directory. Simplest thing would be to create the next numbered shell script. Something like test/02/t065a.sh. Plenty of examples (particularly the more recent ones) to copy and hack. Essentially these just pass known data in and exercise the reading and writing of the format.

The docs and the tests reflect the code-base's Linux roots and if that isn't your normal environment, I'd be happy to help get you over the line.

@danselmi
Copy link
Contributor Author

how can i fix the warnings from cspell which doesn't know "efinix"?

@sierrafoxtrot
Copy link
Owner

how can i fix the warnings from cspell which doesn't know "efinix"?

Just add this to doc/dictionaries/names.txt. Given that we want to give credit for your contribution, I'd suggest adding @danselmi (and your given name & surname if you'd like to list those).

@sierrafoxtrot
Copy link
Owner

Basically if you added doc/man5/srec_efinix.5 the cmake build system should just pick it up automagically. If you were looking for an example, the srec_vmem.5 would be similar and may be familiar if programmed logic is your thing! If you are looking for something complex with diagrams etc, doc/man5/srec_intel.5 and doc/man5/srec_mif.5 are great thorough examples.

[copied over from #52]

The man5 files contain the file descriptions as they are from section 5 "File formats and conventions". The commands themselves are documented in doc/man1/*.1.

If you have something specific to e.g srec_cat then it would go in doc/man1/srec_cat.1

If you have something common (primarily the common input arguments used by srec_cat, srec_info and srec_cmp, then it would go into doc/man1/srec_input.1.

The docs and the tests reflect the code-base's Linux roots and if that isn't your normal environment, I'd be happy to help get you over the line.

This include helping to flesh out any of these docs

@sierrafoxtrot
Copy link
Owner

sierrafoxtrot commented Mar 13, 2023

It looks like you are extremely close to getting the checks to pass so I'll make a start on the actual review. Really keen to get this merged! Also keen for this not to feel like a death-by-review process.

Please let me know if you'd like to me wait (no rush!)

@danselmi danselmi marked this pull request as ready for review March 16, 2023 21:12
Copy link
Owner

@sierrafoxtrot sierrafoxtrot left a comment

Choose a reason for hiding this comment

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

Thanks @danselmi, this is coming along nicely. I think we may have uncovered a bug. If so, it should be easy to tweak. If not, then I'd appreciate if you can educate me a bit more on the file format.

Thanks

srecord/input/file/efinix_bit.cc Outdated Show resolved Hide resolved
srecord/input/file/efinix_bit.cc Outdated Show resolved Hide resolved
doc/man5/srec_efinix.5 Outdated Show resolved Hide resolved
doc/man5/srec_efinix.5 Show resolved Hide resolved
srecord/arglex/tool.cc Show resolved Hide resolved
srecord/input/file/efinix_bit.h Show resolved Hide resolved
srecord/input/file/efinix_bit.h Outdated Show resolved Hide resolved
srecord/input/file/efinix_bit.h Show resolved Hide resolved
test/02/t0265a.sh Outdated Show resolved Hide resolved
doc/man5/srec_efinix.5 Outdated Show resolved Hide resolved
@sierrafoxtrot sierrafoxtrot merged commit 9e3723b into sierrafoxtrot:master Mar 20, 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