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

Switch to read-modify-write flashing. #155

Merged
merged 3 commits into from
Dec 8, 2022

Conversation

denravonska
Copy link
Contributor

This enables programming the chip without having to erase the entire contents. Instead, JLink will erase where needed while retaining the data that's within the same sectors but outside of the specified write area.

I was only able to run some of the Behave tests as I don't have access to the hardware.

Closes #154, #148

@CLAassistant
Copy link

CLAassistant commented Nov 25, 2022

CLA assistant check
All committers have signed the CLA.

@denravonska denravonska changed the title Switch to read-write-modify flashing. Switch to read-modify-write flashing. Nov 25, 2022
@denravonska denravonska force-pushed the feature/program-to-address branch 2 times, most recently from 61b024a to 1147466 Compare November 25, 2022 09:55
@hkpeprah hkpeprah self-requested a review November 30, 2022 16:08
Copy link
Contributor

@hkpeprah hkpeprah left a comment

Choose a reason for hiding this comment

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

LGTM. I'll give it a test run locally, and accept the changes if all good.

pylink/jlink.py Outdated Show resolved Hide resolved
@hkpeprah
Copy link
Contributor

hkpeprah commented Dec 5, 2022

Trying to figure out why the tests are failing with ubuntu-latest, 2.7.

denravonska and others added 2 commits December 5, 2022 12:30
This enables programming the chip without having to erase the entire contents.
Instead, JLink will erase where needed while retaining the data that's within
the same sectors but outside of the specified write area.

Closes square#154, square#148
Co-authored-by: Ford Peprah <hkpeprah@users.noreply.github.com>
hkpeprah
hkpeprah previously approved these changes Dec 5, 2022
Copy link
Contributor

@hkpeprah hkpeprah left a comment

Choose a reason for hiding this comment

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

LGTM. Just one comment from my testing.


return res
bytes_flashed = self._dll.JLINKARM_WriteMem(addr, len(data), data)
Copy link
Contributor

@hkpeprah hkpeprah Dec 5, 2022

Choose a reason for hiding this comment

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

Debugging, it looks like data has to be a bytes vs. a list of integers, so if data is a list of integers, this will fail. I think that means we have to call bytes(data) here.

  • Convert data to bytes if not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you want to retain the signature (list of bytes) and convert to bytes when needed?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think keeping the signature, and convert. What I found was that [1, 2, 3, 4] failed, but bytes([1, 2, 3, 4]) succeeded, so maybe just a simple check like:

if not isinstance(data, bytes):
    data = bytes(data)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made it a bit more strict and only convert if it's a list since that's what the function expects. Type annotations could be useful to soft enforce it.

Not tested yet :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems to be working fine here.

flash() now accepts both list[int] and bytes.
Copy link
Contributor

@hkpeprah hkpeprah left a comment

Choose a reason for hiding this comment

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

LGTM. Will cut a new release later today.

@hkpeprah hkpeprah merged commit feedd6d into square:master Dec 8, 2022
@hkpeprah
Copy link
Contributor

hkpeprah commented Dec 8, 2022

Thanks for the submission. Should be available now in v1.0.0.

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.

Programming without erasing chip
3 participants