wokwi / rp2040js

A Raspberry Pi Pico Emulator in JavaScript
MIT License
410 stars 46 forks source link

feat(ADC): support DMA by setting DREQs based on ADC FIFO fill level #90

Closed tomods closed 2 years ago

tomods commented 2 years ago

The existing ADC and DMA code already had nearly everything – so this just adds some DREQ logic to the ADC code. It is inspired by the PIO DREQ code.

I did some basic tests from Micropython, using the approach from https://iosoft.blog/2021/10/26/pico-adc-dma/ – seems to work fine so far.

Please note that I'm a Javascript/Typescript novice ;)

urish commented 2 years ago

Looks good, thanks!

Mind sharing here the complete test code that you used?

tomods commented 2 years ago

One thing I noticed is that the sample timing using mcu.clock.createTimer() seems to be very inaccurate – like much longer than the configured 2 µs.

I tested this using process.hrtime.bigint() and got times of around 9 milliseconds. Do you have any idea how this could be improved? I might look into this over the weekend – unless I receive my Pico HW tomorrow ;)

tomods commented 2 years ago

Mind sharing here the complete test code that you used?

Unfortunately, it's currently not in a good shape at all – like a combination of Python and Typescript hacks. But I plan to publish my (Python) project as open source :)

tomods commented 2 years ago

Another thing I noticed – I think the simulated ADC code is in need of some better reset logic. Currently, it cannot get out of busy without actually completing an ADC read. I might also look into this soon.

urish commented 2 years ago

Unfortunately, it's currently not in a good shape at all – like a combination of Python and Typescript hacks. But I plan to publish my (Python) project as open source :)

The reason I'm asking - if at some point in the future there someone points out issues with ADC + DMA, we'll have a baseline code that we know works well.

One thing I noticed is that the sample timing using mcu.clock.createTimer() seems to be very inaccurate – like much longer than the configured 2 µs.

Yes, if you need timing accuracy, you should replace the clock with a virtual one. The real time clock can never be accurate enough on a preemptive multi-tasking OS (it might be possible if you pin the process to a specific core, and make sure nothing else runs on the same core, but I doubt anyone will do this).

urish commented 2 years ago

Also, I'm interested: how have you come across rp2040js? Do you have some ideas to build upon it?

tomods commented 2 years ago

The reason I'm asking - if at some point in the future there someone points out issues with ADC + DMA, we'll have a baseline code that we know works well.

OK, makes sense, I'll somehow compile what I did. Not sure what would be the best way to document this – just make it part of this PR? But I probably won't put up much effort for automatizing such a test.

Yes, if you need timing accuracy, you should replace the clock with a virtual one.

That sounds like a good idea. But I'm not sure how I would go about that – and not sure I'm currently interested in digging into this :)

The real time clock can never be accurate enough on a preemptive multi-tasking OS (it might be possible if you pin the process to a specific core, and make sure nothing else runs on the same core, but I doubt anyone will do this).

Well, I'm wondering if there are some "low hanging fruit" for this problem – something like e.g. https://github.com/onury/tasktimer I mean, it doesn't have to be extremely accurate, just somewhat closer.

In general, with a modern Linux kernel, it should be possible to get much better accuracy. However, I'm not sure how well Node lends itself to something like that.

Also, I'm interested: how have you come across rp2040js?

Um, not completely sure any more – it either was by just googling some Pico DMA/ADC details, or by actually searching for something like "raspberry pi pico simulator" because my Pico HW is not arriving and I wanted to test some code ;) What sold me on it was the example https://wokwi.com/arduino/projects/300504213470839309 and of course the Github repo.

(BTW – I think those nice Micropython examples are not reachable directly via the wokwi.com page, only from the README.md.)

Definitely a very pleasant surprise that you took the effort of creating this!

Do you have some ideas to build upon it?

Well, not too much currently to be honest. I'm just using it to run my code in an actually realistic Micropython environment, so for my developing and testing my project code. I find it very impressive that you can run the "official" Micropython port/SDK.

In my test description for ADC/DMA I will also include how I connected the "usual" Micropython development tools to rp2040js. Could be worth it to try and include that in your demo scripts – I'm fairly sure that this could be done directly from Typescript instead of attaching socat to stdin/stdout ;)

tomods commented 2 years ago

OK, I now don't think it would be easy at all to use a better "real" clock here...

urish commented 2 years ago

just make it part of this PR? But I probably won't put up much effort for automatizing such a test.

Thanks! Adding a comment with the code in this discussion can be very useful for anyone who ever touches this code in the future. It's okay not automating the test - we don't have automated tests for all the peripherals.

(BTW – I think those nice Micropython examples are not reachable directly via the wokwi.com page, only from the README.md.)

Good point - I opened wokwi/wokwi-features#265 to track this. And thanks for the feedback!

OK, I now don't think it would be easy at all to use a better "real" clock here...

If you are interested, I can probably come up with an example. It's would probably be similar in nature to the clock implementation we use in AVR8js

tomods commented 2 years ago

Alright, regarding ADC/DMA testing –

Everything I did is based on my project which I just published on https://github.com/tomods/GrinderController. The code links I'm adding in this comment are permalinks including the current commit id, so they hopefully won't get broken in the future.

ADC/DMA handling is done in RP2040ADC.py. The state machine calls capture_start() and wait_and_read_average_u12() every cycle.

To provide some input to the simulated ADC, I just hacked up your demo script, see micropython-run.ts. This is probably not very elegant.

I then used socat to start the simulation and attach its stdin/stdout to a "virtual serial port", see start_socat_rp2040js.sh. This way, I can just use Micropython's pyboard.py or similar tools to start the scripts.

As rp2040js does not support flash/filesystem (yet), I used one more hack – just removing all Python imports internal to my project, and run pyboard.py -d ~/git/rp2040js/serialport rp_devices.py RP2040ADC.py grinder_controller.py grinder_controller_states.py grinder_debouncer.py grinder_hardware.py main.py to actually run my code.

The debug logging my code currently does will then output the averaged ADC values coming in via DMA every 10 cycles. So then I just looked at those and manually compared them to the values provided in my micropython-run.ts – and they are in fact plausible :)

Before applying the changes, DMA would – quite obviously – hang indefinitely. Ideally, this test would be extended to really test that each and every ADC value is read/copied, but I currently don't see a big risk here.

You can also find my timing test code using process.hrtime.bigint() hacked into micropython-run.ts. It just dumps the time between consecutive adc.onADCRead() calls into a file.

urish commented 2 years ago

Alright, this seems to work well. Thanks Tobias!

Released as rp2040js 0.14.8, also made a Wokwi test project based on your code:

https://wokwi.com/arduino/projects/320232834674459219

tomods commented 2 years ago

Nice, you're welcome!

By the way – did I see correctly that your simulator web "frontend" is not open source? At least I couldn't find it. Would be interested in learning how you did the file handling – especially for the Python scripts – and how the simulated inputs/output are connected to rp2040js.

urish commented 2 years ago

did I see correctly that your simulator web "frontend" is not open source?

True

how you did the file handling

There are some details here (for read-only access):

https://github.com/wokwi/rp2040js/issues/81#issuecomment-932823793

And if you wish to make the virtual flash filesystem writable as well, check out the discussion in #88

urish commented 2 years ago

p.s. GrinderController sounds like a cool project.

Instead of measuring the battery voltage, how about measuring the current? This can be done directly (by adding a low-resistance power resistor in series), or indirectly (with an hall-effect current sensor)

tomods commented 2 years ago

Thanks for checking out my project :)

Measuring the current – more or less directly – was my first thought for the project, and is currently my "backup plan". Basically, I'd like to keep things as simple as possible, especially regarding hardware components.

So, my current plan is to try it out using the battery voltage measurement only – if that fails, I will probably try to add a shunt resistor to measure the current. Thanks for the hint about other types of current sensors!

tomods commented 2 years ago

There are some details here (for read-only access):

#81 (comment)

That worked :)

Would you be interested in a PR for micropython-run.ts that allows loading a littefs image file into the simulated flash? I decided on creating the image file outside of the Typescript world, and then just read this. But this could later be "upgraded" to actually generate the image file from Typescript.

urish commented 2 years ago

Perhaps it's better to have a different example, micropython-fs.ts or so which allows loading an image into the simulated flash, as it's a bit more advanced use caes. Other than that, such PR is welcome (you can also mention the issue in a comment, for anyone who want to create the image directly in TypeScript)

tomods commented 2 years ago

(littlefs stuff continued at https://github.com/wokwi/rp2040js/pull/92)