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

Set board property on AVRRunner class? #67

Closed
elliottkember opened this issue Nov 15, 2020 · 10 comments
Closed

Set board property on AVRRunner class? #67

elliottkember opened this issue Nov 15, 2020 · 10 comments
Labels
enhancement New feature or request

Comments

@elliottkember
Copy link

elliottkember commented Nov 15, 2020

The execute.ts function differs between the two examples (mega and normal boards) - this always trips me up between examples.

It seems to be because the new CPU has an offset second parameter:

this.cpu = new CPU(this.program, 0x2200);

Would it be possible to set board type on AVRRunner, and then do the offset depending on what the board is set to, so the user can just use the AvrRunner directly from avr8js without redeclaring it?

import { AVRRunner } from "avr8js";
new AVRRunner(hex, { board: 'mega' });

I also don't really know what portB/portC/portD refer to. I wonder whether the AVRRunner class could expose a pin(int pinNumber) function that calls portB.pinState(X) internally.

const arduino = new AVRRunner(hex, { board: 'mega' });
const ledController = new WS2812Controller(cols * rows);
// Send pin 6 state to the matrixController
arduino.addPinListener(6, (state) => {
  ledController.feedValue(state, cpuNanos())
});

Or even tidier, the WS2812Controller could set up the listener itself:

// pin 12 is the pin set in the FastLED code example
ledController.setInputPin(arduino, 12);

Thanks!

@urish urish added the enhancement New feature or request label Nov 16, 2020
@urish
Copy link
Contributor

urish commented Nov 16, 2020

Hi Elliot!

That's a good piece of feedback! Let's address each point individually:

  1. Differences between different chips: At first, the project's goal was to provide a working simulation of ATmega328p, that's the chip that you find on Arduino Uno boards. Eventually, I started to extend it to support other chips in the AVR family, such as the ATmega2560 (the chip on the Arduino MEGA), and now also ATtiny25/45/85. The idea is to provide the basic building blocks, so everyone can mix and match to create various chip configurations, and even ones that do not exist as a physical device. One such example is creating an ATmega2560 with 32kb of SRAM, which can drive up to 10,000 LED matrices. The ATmega2560 simulation is usuable, but is far from being complete - there's much effort to go into mapping all the peripherals (e.g. multiple serial ports, I2C ports, etc), and creating the configuration objects for all of them.

  2. At the current point, we have presets for individual peripheral configuration (e.g. timer0Config, portBConfig, etc.), and they are all modeled after the ATmega328p. I imagine we could create some presets for different chip types which contain settings such as RAM size, FLASH size, EEPROM size, signature bytes (which we currently don't emulate), and probably also peripheral configurations. That could be useful for people who are looking to quickly get up and running.

  3. As for including the AVRRunner in the library: Hiding too much of the internal details will make it hard to customize the simulation while keeping the performance high. So there's a tradeoff here, and that's why I'm reluctant to include AVRRunner in the core package - it is very opinionated about how to wire the hardware. For instance, in your case, you probably don't need the 4 USART hardware ports that come with the ATmega2560, nor the 6 hardware timers, so why would you want to pay the performance penalty for simulating those?

  4. As for what portB / portC / portD refer to, I received similar feedback from the @andre-dietrich who is integrating AVR8js in LiaScript. These refer to the names of the pins on the actual ATmega328p chip (PB0, PB1, etc.). Arduino has a different numbering scheme that abstracts the specifics of a chip, and you can find the complete pin mapping in their source code. There's even a nice ASCII art that shows the physical chip and the names of the pins. I assume we could export some kind of mapping between Arduino pins and ATmega328p pins as a utility, but if you don't include AVRRunner in the core library, you'd still have to write some code to use that mapping.

Thoughts?

@elliottkember
Copy link
Author

This all sounds great. This is definitely a much larger project than just the WS2812B emulation I'm using it for.

I think the real answer here is probably for me to make a separate little wrapper library that just does the LED stuff I want. That way I can match it with a version of avr8js and keep the execution code out of the apps. I use the runner in a couple of places, including on the server for generating GIFs, so it'll be worth it to have a consistent interface.

For instance, in your case, you probably don't need the 4 USART hardware ports that come with the ATmega2560, nor the 6 hardware timers, so why would you want to pay the performance penalty for simulating those?

Good question! I don't actually know whether I am emulating them in my execute.ts, and I didn't know there was a performance penalty. Is that these lines?

this.portB = new AVRIOPort(this.cpu, portBConfig);
this.portC = new AVRIOPort(this.cpu, portCConfig);
this.portD = new AVRIOPort(this.cpu, portDConfig);

@elliottkember
Copy link
Author

elliottkember commented Nov 18, 2020

☝️ The API I have so far for such a wrapper library looks like this:

This is now an NPM module and so far it's working!

import LEDuino, { dataPin } from "@elliottkember/leduino";

const leduino = new LEDuino({
  rows: 14,
  cols: 14,
  serpentine: true,
  hex: build.hex,
  canvas: document.getElementById('canvas'),
  // onPixels: (pixels) => console.log(pixels.length),
  // onSerial: (text) => console.log(text),
});

And it exports the pin for the precompiled FastLED code: #export LED_PIN ${dataPin}

This covers all my use-cases and will make it super easy for me to update AVR8js in future across all the places I use it :)

@urish
Copy link
Contributor

urish commented Nov 23, 2020

this.portB = new AVRIOPort(this.cpu, portBConfig);
this.portC = new AVRIOPort(this.cpu, portCConfig);
this.portD = new AVRIOPort(this.cpu, portDConfig);

Shouldn't affect the performance too much - basically, any call to .tick() methods (or any other code that you put into the execute() loop) can affect performance, since it runs for every single opcode in the program, or about 8,000,000 times per second. For instance, this is how I improved the timer performance from commit 673cbaf: I removed a relatively expensive object lookup from the tick() method, and cached the result in an instance variable.

This is now an NPM module and so far it's working!

Super cool! I see you already got to version 1.0.5. Do you need some help with crafting a README?

@elliottkember
Copy link
Author

Super cool! I see you already got to version 1.0.5. Do you need some help with crafting a README?

Thank you! Yes I made a few small patch changes in the process. For now I'm going to wait on the README until I'm confident the API won't change. I think it's pretty close.

@urish
Copy link
Contributor

urish commented Dec 20, 2020

How is it going with that Elliot?

@elliottkember
Copy link
Author

elliottkember commented Dec 30, 2020

How is it going with that Elliot?

I got this working, and then completely forgot about it! Which is a good sign as it worked great.

Do you think anybody else will have a need for such a wrapper library? I'm not sure whether it's worth putting more time into documenting or maintaining it as I don't think anybody will have a need to use it.

I think the AVR8js library is amazing, and I'm very grateful for it! But I have found it very complicated to work with. Little libraries like the one I've written will be extremely helpful for helping developers like me use it for specific things.

At any rate, I think we can close this issue now :)

@urish
Copy link
Contributor

urish commented Jan 1, 2021

I got this working, and then completely forgot about it! Which is a good sign as it worked great.

Happens to me sometimes too :-)

I think the AVR8js library is amazing, and I'm very grateful for it! But I have found it very complicated to work with. Little libraries like the one I've written will be extremely helpful for helping developers like me use it for specific things.

That's a very good piece of feedback. I've been thinking this over the last few days, and I think we'll have an easier API for people who just want to get ATmega328p (and eventually, maybe also Uno) simulation out of the box.

The latest release, 0.14.x, has already simplified the API a little: you no longer have to call .tick() on each of the peripherals, the CPU class manages this for you (and this comes with nice performance gains, since it's smart enough to execute the peripheral code only when needed).

This paves the way to creating a preset - some kind of factory function that returns a CPU instance with all the peripherals preconfigured. e.g. something like:
createAVR(config), where config will include information about the flash size, ram size, eeprom size, etc. So eventually setting up the simulation can be something along the lines of:

const { cpu } = createAVR(ATmega328p);

// this code runs in a loop
while (...) {
  avrInstruction(cpu);
  cpu.tick();
}

urish added a commit that referenced this issue Jan 1, 2021
@urish
Copy link
Contributor

urish commented Jan 1, 2021

The simpler-api branch now has a prototype implementation

urish added a commit that referenced this issue Feb 20, 2021
urish added a commit that referenced this issue Mar 5, 2021
@urish
Copy link
Contributor

urish commented Mar 5, 2021

Moving the discussion to #87

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants