-
Notifications
You must be signed in to change notification settings - Fork 595
Dev #79
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
Dev #79
Conversation
The eol character is now configurable The user can now actuvate echo feedback to see what he is typing I also removed a duplication in two run() overload, while fixing what I think was a mistake by enforcing the use of the provided argument Stream& even if the ctor one is not null. Lastly, I warn the user if \n is configured but \r is detected (only in user friendly mode) All the past code should work as before (no API change required)
I use -D SIMPLEFOC_SAMD_DEBUG_SERIAL=SERIAL_PORT_MONITOR
I renamed the enum member EXTERN to EXTERNAL. For consistency, I also renmaed INTERN.
The solution is to define them based on the equivalent rp2040 constants
until I find a way to make it compatible, it is disabled when compling for rp2040. Problem is that SPIClass and SPISettings are not defined, SPI seems to work somewhat differently in rp2040
Issue simplefoc#46: motor.command() and CRLF
No need, the other part of the code is merged already. @runger1101001 are you ok with this serial modification? |
Basically the SAMD stuff is fine, but there are a lot of changes in Commander that I am uncertain about... I think unfortunately you guys were working on that in parallel. At this point the way I would do it is to merge the changes over manually (locally), so that I can exclude the commander changes. I am more than happy to do this, but if you @maxlem would prefer to send another pull request excluding the commander changes, then GitHub would track that correctly. If I do it locally it will look like the change came from me... |
…centered readings with the drv8305, just as intented
…uino due same frequency PWM and TC calculation
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.
Basically perfect, except we have parallel changes in Commander.cpp which can't be merged. Without the commander changes I would merge it!
I use -D SIMPLEFOC_SAMD_DEBUG_SERIAL=SERIAL_PORT_MONITOR
…centered readings with the drv8305, just as intented
Well, no more conflicts, but it's a mess. I simply forgot to create a branch when I started to work on ADC-DMA stuff, it was a different feature. In a perfect world, I would scrap all this and start a new fork, cherry-picking all the changes I really want to make it into dev I'm so sorry :( Tell me what you'd like I'll do it. We cand always leave the ADC/LowSide stuff as it is since we're in a dev branch, but while the ADC (current_sense/hardware_specific/samd21_mcu.h) class is in a usable state, the LowSideCurrentSense.h is plain wrong, returns volts instead of amps right now and I know @askuric is working on its own |
Wow, now it says 51 files changed 😱 So I checked it out locally and applied it to the clean dev branch, and it changes only 10 files, and only the SAMD-related stuff. It is a FF merge. So I will merge it. It can be reversed if it turns out different online than when I do it locally... |
Perfect, it worked out on GitHub just the same way as locally. Thank you so much, @maxlem for your hard work! If I make make a suggestion, please rename your current "dev" branch (e.g. "dev-old") and create a new "dev" branch from the current simplefoc:dev branch. In this way your (future) changes will be easy to include. |
this contains my other MR which askuric says he's fine with it
#75
let me know if you want me to split them in two