-
Notifications
You must be signed in to change notification settings - Fork 5
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
The switch Voltage → Current control is NOT bumpless #86
Comments
I don't have a suitable simulation infrastructure (either harness or ad-hoc test module) for testing this. Postponed to the next sprint and to be discussed with @mfussi66. |
Hi @mfussi66 Last week, I had a quick look at all the state machines involved and, apparently, the setting of the proper current setpoint that guarantees a bumpless transition should be handled as expected, if I recall it right. As said though, I don't have the simulation infrastructure to carry out the tests on my own. /remind Thursday let's talk about this F2F. |
⏰ Reminder
|
|
If it won't be possible to discuss it F2F, I can work on this issue in this sprint. |
Sorry @mfussi66, I had a setback today. At any rate, discussing how to complete the simulation machinery is helpful. |
After a F2F alignment, the plan is that @mfussi66 will debug the problem on a physical setup. I'll put the issue on the next sprint, but feel free to reschedule it.
Simply a harness created at the architecture level should suffice. A surrounding machinery should be needed anyhow, which can be probably copied out from |
Great! It should be: |
Ah yes, when I made the refactor of the dictionary, I missed the Iq filtered defined as bus. I'll correct it, but I'm pretty sure it is not part of the problem. This implies that I tackled this issue with the experimental amcbldc code, but it's better if I switch to the official one. |
Yes, I was just pointing it out.
Yep, definitely. We should stick to the whole official framework for the fix. |
I think at the end of the day, the culprit is the experiment_supervisor. This behaviour does not occur when a ETH board communicates with the AMCBLDC, but I wonder if it is a hidden bug that might have been triggered by this non-standard behaviour. |
Nice debuggging!
I kind of concur that the system should be robust against things unforeseen by the protocol. That being said though, this is a general remark that holds for the whole architecture and not just for this specific case. Thus, we may consider ourselves satisfied here. I would only adapt the |
A simple change on the logic of the Periodic message trigger fixes the error. I added a "reset on change system", so that for one Model Ts the CAN Transmit would stop sending messages periodically. Implementation: For some reason, keeping the periodic transmission enabled would keep sending the same messages over and over. Now it is fixed: as soon as the message type changes from |
PR: #91 |
PR merged! |
Given that this workflow is very unlikely to happen, it might be useful to notify it in a separate issue with low priority.
MATLABWindow_hnlp1F7PTL.mp4
Originally posted by @mfussi66 in #84 (comment)
The text was updated successfully, but these errors were encountered: