-
Notifications
You must be signed in to change notification settings - Fork 7
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
Firmata building with Yotta on nRF52833 #7
Conversation
Firmata hex file for V2 |
"dependencies": { | ||
"microbit": "lancaster-university/microbit#v2.1.1" | ||
"codal-microbit": "microbit-foundation/codal-microbit" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we switch this and the previous dep to both be targetDependencies so we can change which dal we want based on the target.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you intend to change the license to Apache 2.0? Even if Codal is Apache 2.0, micro:bit Firmata can be MIT; they are compatible licenses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jhmaloney. Just FYI, Codal is licensed as MIT...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not intentionally, possibly defaulted to it when I yotta init
'd the directory. I'll correct that in the next commit!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to know!
MICROBIT_PIN_P8, MICROBIT_PIN_P9, MICROBIT_PIN_P10, MICROBIT_PIN_P11, | ||
MICROBIT_PIN_P12, MICROBIT_PIN_P13, MICROBIT_PIN_P14, MICROBIT_PIN_P15, | ||
MICROBIT_PIN_P16, /* 17-18 */ MICROBIT_PIN_P19, MICROBIT_PIN_P20); | ||
MicroBit uBit; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the issue with using uBit here is that we'll run out of RAM on a V1?
Which component was the problem here?
It is no bad thing having the uBit component but @jhmaloney do you remember why you were using indivudual components? @microbit-sam does this still build for V1 like this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't remember, but I doubt this choice was driven by running out of RAM. It may have been simply a desire to use the minimal set of DAL features required. I have wanted to avoid competing with DAL processes for either CPU cycles or for certain hardware resources such as sensors.
You understand the internals of the DAL better than I do so if you think it makes sense to use uBit go for it. You can use the test suite to see if anything breaks. :-)
|
||
// Variables | ||
|
||
#define MBED_LIBRARY_VERSION 0 | ||
#define MICROBIT_DAL_VERSION "CODAL" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should then be target dependent too.
We can close this as CODAL support has been added with PR #12 |
No description provided.