-
Notifications
You must be signed in to change notification settings - Fork 18.3k
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
SerialManager IOMCU fixups #29475
base: master
Are you sure you want to change the base?
SerialManager IOMCU fixups #29475
Conversation
9e9157c
to
6faa4e4
Compare
@@ -184,7 +184,7 @@ const AP_Param::GroupInfo AP_SerialManager::var_info[] = { | |||
// @DisplayName: Telem1 protocol selection | |||
// @Description: Control what protocol to use on the Telem1 port. Note that the Frsky options require external converter hardware. See the wiki for details. | |||
// @SortValues: AlphabeticalZeroAtTop | |||
// @Values: -1:None, 1:MAVLink1, 2:MAVLink2, 3:Frsky D, 4:Frsky SPort, 5:GPS, 7:Alexmos Gimbal Serial, 8:Gimbal, 9:Rangefinder, 10:FrSky SPort Passthrough (OpenTX), 11:Lidar360, 13:Beacon, 14:Volz servo out, 15:SBus servo out, 16:ESC Telemetry, 17:Devo Telemetry, 18:OpticalFlow, 19:RobotisServo, 20:NMEA Output, 21:WindVane, 22:SLCAN, 23:RCIN, 24:EFI Serial, 25:LTM, 26:RunCam, 27:HottTelem, 28:Scripting, 29:Crossfire VTX, 30:Generator, 31:Winch, 32:MSP, 33:DJI FPV, 34:AirSpeed, 35:ADSB, 36:AHRS, 37:SmartAudio, 38:FETtecOneWire, 39:Torqeedo, 40:AIS, 41:CoDevESC, 42:DisplayPort, 43:MAVLink High Latency, 44:IRC Tramp, 45:DDS XRCE, 46:IMUDATA, 48:PPP, 49:i-BUS Telemetry | |||
// @Values: -1:None, 1:MAVLink1, 2:MAVLink2, 3:Frsky D, 4:Frsky SPort, 5:GPS, 7:Alexmos Gimbal Serial, 8:Gimbal, 9:Rangefinder, 10:FrSky SPort Passthrough (OpenTX), 11:Lidar360, 13:Beacon, 14:Volz servo out, 15:SBus servo out, 16:ESC Telemetry, 17:Devo Telemetry, 18:OpticalFlow, 19:RobotisServo, 20:NMEA Output, 21:WindVane, 22:SLCAN, 23:RCIN, 24:EFI Serial, 25:LTM, 26:RunCam, 27:HottTelem, 28:Scripting, 29:Crossfire VTX, 30:Generator, 31:Winch, 32:MSP, 33:DJI FPV, 34:AirSpeed, 35:ADSB, 36:AHRS, 37:SmartAudio, 38:FETtecOneWire, 39:Torqeedo, 40:AIS, 41:CoDevESC, 42:DisplayPort, 43:MAVLink High Latency, 44:IRC Tramp, 45:DDS XRCE, 46:IMUDATA, 48:PPP, 49:i-BUS Telemetry, 50: IOMCU |
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.
serial manager does not control the IOMCU uart, so we should not have the parameter exposed. If the parameter is exposed it should do something
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.
fixed here #29493
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.
so is this going to be removed? if not, put a WIKI NEEDED label on the PR please
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.
Since the param is still exposed on CubeRed we need the value in the description.
@bugobliterator can you please check? did you deliberately make SERIAL7 appear in param list for CubeOrangePlus and other IOMCU boards? That is a bad user experience showing parameters that do nothing |
@tridge yes that unintentional. Here is a fix that should solve this issue. #29493 |
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.
the stats changes are correct, except that all the rest of the changes will break CubeRed workflow.
The current behaviour is that the serial protocol is set to IOMCU for internal use only, it shouldn't have been exposed for other boards, that's oversight on my part. I have a fix for that here #29493
For cubered this parameter is defaulted to PPP but if BRD_IO_ENABLE is set its changed to IO Protocol in AP_Vehicle, that is the desired behaviour. And the serial is visible in parameters as well, for users to modify as well.
@@ -184,7 +184,7 @@ const AP_Param::GroupInfo AP_SerialManager::var_info[] = { | |||
// @DisplayName: Telem1 protocol selection | |||
// @Description: Control what protocol to use on the Telem1 port. Note that the Frsky options require external converter hardware. See the wiki for details. | |||
// @SortValues: AlphabeticalZeroAtTop | |||
// @Values: -1:None, 1:MAVLink1, 2:MAVLink2, 3:Frsky D, 4:Frsky SPort, 5:GPS, 7:Alexmos Gimbal Serial, 8:Gimbal, 9:Rangefinder, 10:FrSky SPort Passthrough (OpenTX), 11:Lidar360, 13:Beacon, 14:Volz servo out, 15:SBus servo out, 16:ESC Telemetry, 17:Devo Telemetry, 18:OpticalFlow, 19:RobotisServo, 20:NMEA Output, 21:WindVane, 22:SLCAN, 23:RCIN, 24:EFI Serial, 25:LTM, 26:RunCam, 27:HottTelem, 28:Scripting, 29:Crossfire VTX, 30:Generator, 31:Winch, 32:MSP, 33:DJI FPV, 34:AirSpeed, 35:ADSB, 36:AHRS, 37:SmartAudio, 38:FETtecOneWire, 39:Torqeedo, 40:AIS, 41:CoDevESC, 42:DisplayPort, 43:MAVLink High Latency, 44:IRC Tramp, 45:DDS XRCE, 46:IMUDATA, 48:PPP, 49:i-BUS Telemetry | |||
// @Values: -1:None, 1:MAVLink1, 2:MAVLink2, 3:Frsky D, 4:Frsky SPort, 5:GPS, 7:Alexmos Gimbal Serial, 8:Gimbal, 9:Rangefinder, 10:FrSky SPort Passthrough (OpenTX), 11:Lidar360, 13:Beacon, 14:Volz servo out, 15:SBus servo out, 16:ESC Telemetry, 17:Devo Telemetry, 18:OpticalFlow, 19:RobotisServo, 20:NMEA Output, 21:WindVane, 22:SLCAN, 23:RCIN, 24:EFI Serial, 25:LTM, 26:RunCam, 27:HottTelem, 28:Scripting, 29:Crossfire VTX, 30:Generator, 31:Winch, 32:MSP, 33:DJI FPV, 34:AirSpeed, 35:ADSB, 36:AHRS, 37:SmartAudio, 38:FETtecOneWire, 39:Torqeedo, 40:AIS, 41:CoDevESC, 42:DisplayPort, 43:MAVLink High Latency, 44:IRC Tramp, 45:DDS XRCE, 46:IMUDATA, 48:PPP, 49:i-BUS Telemetry, 50: IOMCU |
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.
fixed here #29493
@bugobliterator What is the plan with the serial port params on CubeRed? Are the only valid values IOMCU or PPP? In which case I don't think you need to expose the params, the switch can be done on only board IO enable param? If the params are exposed setting board IO enable should set and default the parameters. My goal with adding the defaults is to make sure that parameters that the user did not change don't show up as not default values in the log. |
Actually for CubeRed primary secondary comms can be DDS and mavlink as well, nothing is set in stone there, and would like to keep all the options open there. As far as the change to not show up as non-default value, I guess we can change the default value of the parameter instead of just setting the value itself. |
6faa4e4
to
aad9115
Compare
@bugobliterator Thanks for the explination. I have added what I think is missing from #29493 and removed everything else. The only thing that remains is there is no way for the hardware report WebTool to know which logged UART instance is the IOMCU. I had it as a special case of 100 but that was removed in #29094. For all the other UARTs I can lookup the params based on the index to find the protocol. After #29493 that method won't work on the IOMCU anymore. We could hard coded it back to 100 for the instance which equals |
@IamPete1 I think we can simply add protocol field to uart_info.txt. Where it can show which protocol its currently set to if available. That would be more informative long-term as well. |
Fix-ups for #29094.
The IOMCU now shows up as a full serial port with parameters (CubeOrangePlus), so the param docs should be added so the user knows what 50 is. This also adds defines for the default baud and protocol so the params don't show up as changed by the user.
I'm not sure if the intention was that the params should show up at all on boards other than CubeRed?
There is also a now unused stats tracker that can be removed now the IOMCU is in the main serial array.