-
Notifications
You must be signed in to change notification settings - Fork 34
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
Bug in LPF2Hub.cpp #64
Comments
Hi Thorsten (@ThorstenBenter)! Many thanks for reaching out via GitHub 👍 I will check about the different commands for the Tacho motors. Very good finding! On the time i have implemented this i was not really able to test it with the appropriate hardware. All the best, |
Hi Cornelius (@corneliusmunz ), |
Hi Cornelius (@corneliusmunz) (1) Changes.txt is just my personal log to keep track of things I did to original files I guess this is all I have. All the best, |
Hi Thorsten! (@ThorstenBenter), Regarding the colors I will change this in a "breaking" release and will introduce a struct enum where you can define the color like in other struct enums "Lpf2::RED" etc. The PR from @fredlcore is already merged into the master branch and I have added the isScanning keyword afterwards as you have suggested it. Here you can find the feature branch with the changes I have made related to your issue: https://github.com/corneliusmunz/legoino/tree/feature/issue_64_wrong_motor_command It would be great if you could do a review on the changes and could test if this will solve your issues. I would highly appreciate this. All the best and a littlebit late a happy new year 🥳 |
@ThorstenBenter: cool project! And even cooler that you used your Specci :) - I'm an 8-bit enthusiast as well (coming from Atari), and maybe just as an idea if you want to open your project for people without the interface 1: it's fairly easy to communicate with an Arduino/ESP from an 8-Bit computer using any kind of joystick port or user interface. I've made a little project called AtariDuino for that, maybe it's helpful if you want to think in that direction: Is the ZX code in BASIC or Assembly? The former would be easy to port to the Atari and other 8-Bit systems, Z80 code not so much ;)... |
Hi Cornelius! (@corneliusmunz) |
@fredlcore Thank you very much! |
Hi Cornelius,
I tried to get in touch with you on the Eurobricks Forum (https://www.eurobricks.com/forum/index.php?/forums/topic/180905-new-version-100-of-legoino-arduino-library/&do=findComment&comment=3478705) - but I am sure you have simply so many other things to do.
Just to make sure: Legoino is my absolutely favorite software, when it comes to PF and PF2 control. It does not get >any< better. I have no clue how you accomplished this besides job and life. Thank you again so much for providing this to the community. With your help and efforts , I was able to make a dream come true (https://www.eurobricks.com/forum/index.php?/forums/topic/188584-mulpi-a-multiple-lego-remote-protocol-interface/).
I believe there are two minor bugs in LPF2Hub.cpp, please see the first Eurobricks link above for details.
In short:
[I believe it is favorable - even in the function names - to distinguish "power" from "speed". TLG does that in their totally outdated LWP3.0 protocol document on GitHub as well. That is just naming, but will break code, but there are very simple workarounds]
(1) For the setTachoMotorSpeed function, you are using 0x01 as motor sub-command of the LWP3.0 0x81 Output Command. I believe this needs to be 0x07 [StartSpeed (Speed, MaxPower, UseProfile)] to work correctly.
(2) The StartSpeed sub-command does not have any BrakeStyle option - as it is controlling the speed. All other regulated (speed) commands do have that option and work beautifully well!
(3) Once the setTachoMotorSpeed function is changed to work with sub-command 0x07, the stopTachoMotor function does not work anymore, as there the speed setting 0 does cause the function to behave weird (it ramps the power to 100%). The LWP3.0 protocol says that when you want to set the speed of a tacho motor to 0, you have to use StartPower(port, 0(brake)/127(float)) to stop the motor.
For the setBasicMotorSpeed, you are using sub-command 0x51. I believe, it would be more consistent to use 0x01 (StartMotorPower). But: That is just a matter of taste, but not functionality. 0x51 works perfectly well.
That's all - just very minor things. I have tested the changed cpp file extensively and - it seems to work.
All the best,
Thorsten
The text was updated successfully, but these errors were encountered: