Provide estimated power, duration, distance and coverage on mission planning#2123
Conversation
|
Amazing work. |
686b2c7 to
495a476
Compare
|
@rafaellehmkuhl modifications done! ready for review |
src/stores/mission.ts
Outdated
| const setSurveyAreaM2 = (id: string, areaM2: number): void => { | ||
| surveyAreaM2ById.value = { ...surveyAreaM2ById.value, [id]: areaM2 } | ||
| } | ||
|
|
||
| const removeSurveyAreaM2 = (id: string): void => { | ||
| const next = { ...surveyAreaM2ById.value } | ||
| delete next[id] | ||
| surveyAreaM2ById.value = next | ||
| } | ||
|
|
||
| const clearAllSurveyAreas = (): void => { | ||
| surveyAreaM2ById.value = {} | ||
| } |
There was a problem hiding this comment.
Those methods are only used in the useMissionStatistics composable. It's better to keep them there, as this data is not shared across any other components.
Also, can we rename them to setSurveyAreaSquareMeters and removeSurveyAreaSquareMeters?
| const turnPenaltySeconds = computed(() => { | ||
| const wps = missionStore.currentPlanningWaypoints || [] | ||
| let secs = 0 | ||
| if (wps.length >= 2) secs += 1 // adds deceleration time | ||
| for (let i = 1; i < wps.length - 1; i++) { | ||
| const prev = wps[i - 1].coordinates as LatLng | ||
| const cur = wps[i].coordinates as LatLng | ||
| const next = wps[i + 1].coordinates as LatLng | ||
| const diff = deltaBearing(bearingBetween(prev, cur), bearingBetween(cur, next)) | ||
| if (diff > 45) secs += 1 + 1 + Math.ceil(diff / 15) | ||
| } | ||
| return secs | ||
| }) | ||
|
|
||
| // Vehicle parameter input | ||
| const speedMps = computed<number>(() => { | ||
| const speed = Number((missionStore as any).defaultCruiseSpeed) | ||
| return speed > 0 ? speed : 1 | ||
| }) | ||
|
|
||
| // Extra payload in kg | ||
| const payloadKg = computed<number>(() => { | ||
| const payload = Number(vehicleStore.vehiclePayloadParameters?.extraPayloadKg) | ||
| return Number.isFinite(payload) && payload >= 0 ? payload : 0 | ||
| }) | ||
|
|
||
| // Battery mass & capacity from count | ||
| const batteryMassKg = computed<number>(() => { | ||
| const battCapacity = Number(vehicleStore.vehiclePayloadParameters?.batteryCapacity) || 2 * 266.4 // default battery pack capacity | ||
| const chem = (vehicleStore.vehiclePayloadParameters?.batteryChemistry ?? 'li-ion') as BatteryChemistry | ||
| const kgPerWh = kgPerWhByChem[chem] ?? kgPerWhByChem['li-ion'] | ||
| return battCapacity * kgPerWh | ||
| }) | ||
|
|
||
| // Power estimate | ||
| const powerW = computed(() => { | ||
| const originalBatteryMassKg = 1.152 * 2 // default battery pack mass | ||
| const totalExtraMass = payloadKg.value + (batteryMassKg.value - originalBatteryMassKg) | ||
| const pAt1 = Math.max(5, basePowerAtOneMs + massSlopeWPerKg1Mps * totalExtraMass) | ||
| const speed = Math.max(0.1, speedMps.value) | ||
| return pAt1 * Math.pow(speed / 1.0, speedExponent) | ||
| }) | ||
|
|
||
| // Estimate mission time with added penalties | ||
| const missionETASeconds = computed(() => { | ||
| if (speedMps.value <= 0) return NaN | ||
| return missionLengthMeters.value / speedMps.value + turnPenaltySeconds.value | ||
| }) | ||
|
|
||
| // Energy consumption estimate (Wh) | ||
| const missionEnergyWh = computed(() => { | ||
| if (!isFinite(missionETASeconds.value)) return NaN | ||
| return (powerW.value * missionETASeconds.value) / 3600 | ||
| }) |
There was a problem hiding this comment.
Can we move those methods that are estimations based on vehicle parameters to a dedicated .ts file? This way it is much easier for someone extending this functionality in the future (e.g.: for aerial drones) without messing with UI-related code (as this composable).
There was a problem hiding this comment.
Oops. Indeed it could to be moved
I think this behavior is the same that we had before this feature was implemented. Check out the video below, where I start creating a survey the same way you did, and it goes like on blind-mode, until the third vertex is added to the polygon. From that point on, every extra click adds an extra vertex, as it is on master: Maybe what is happening is that we are now providing an extra reference to the polygon creation and we naturally expect the creation process to have more extra references than that. And about the waypoints not being created bug: Its not a bug, what you saw was caused by the size of the polygon you drew. It was a square with less than 10 meter each side, so with the distance between survey lines being set to 10 meters, barely no line would be created. survey.creation.mp4 |
It makes sense! Tested again and the problem was indeed the size of the survey. |
495a476 to
d5f85f1
Compare
All considerations addressed! |
Did you take a look at these ones? It seems like they are as before. |
24a24f2 to
53ba4be
Compare
Thanks for the remind. It's indeed way better to have those vehicle specific methods separated from the rest. Ready for review! |
|
Added on Latest patch:
|
rafaellehmkuhl
left a comment
There was a problem hiding this comment.
From an architectural point of view, the blueboat-estimates.ts should not be a Vue file, specially a composable, as it makes extending it to other vehicles a difficult and messy task. It is today accessing data from all over the codebase, so it's opening a new star of dependencies. Vue files should only be implementing UI logic, not business logic.
Ideally there should be a vehicle-estimates interface that specifies what is passed to the estimator and what is returned, and the blueboat-estimates.ts file would simply export an implementation of this interface (an object with the necessary methods, their parameters and returns), and the MissionStatistic.vue would simply be calling those methods (e.g.: timeToCompleteMission and totalEnergy) with the necessary parameters. This would also make so we do not export unneeded data from this, as missionEnergyWh and missionETASeconds . Something like the following:
interface VehicleMissionEstimate = {
timeToCompleteMission: (waypoints: Waypoints, missionConfig: any) => number,
totalEnergy: (waypoints: Waypoints, missionConfig: any) => number
}
export const blueBoatMissionEstimate: VehicleMissionEstimate = {
timeToCompleteMission: (waypoints: Waypoints, missionConfig: any): number => {
...
},
totalEnergy: (waypoints: Waypoints, missionConfig: any): number => {
...
}
}
// This one is just an example on how easy it gets to implement new ones if we implement a good interface
export const otherGenericVehicleMissionEstimate: VehicleMissionEstimate = {
timeToCompleteMission: (waypoints: Waypoints, missionConfig: any): number => {
...
},
totalEnergy: (waypoints: Waypoints, missionConfig: any): number => {
...
}
}
let timeToCompleteMission: number | undefined = undefined
let totalEnergy: number | undefined = undefined
if (vehicle == 'blue-boat') {
timeToCompleteMission = blueBoatMissionEstimate.timeToCompleteMission(...)
totalEnergy = blueBoatMissionEstimate.totalEnergy(...)
} else if (vehicle == 'generic-vehicle-with-estimates') {
timeToCompleteMission = otherGenericVehicleMissionEstimate.timeToCompleteMission(...)
totalEnergy = otherGenericVehicleMissionEstimate.totalEnergy(...)
} else {
throw Error(`Estimates for vehicle ${vehicle} were not implemented`)
}
Do you think it would be possible to change it to something like that? I think it can be done quickly and really helps us in the future.
One small UI problem I noticed is this blinking panel:
Kapture.2025-09-26.at.16.29.09.mp4
| </select> | ||
| <p class="m-1 overflow-visible mt-2 text-sm text-slate-200">Default cruise speed (m/s)</p> | ||
| <input v-model="defaultCruiseSpeed" class="px-2 py-1 mt-1 mb-2 mx-5 rounded-sm bg-[#FFFFFF22]" /> | ||
| <v-divider class="my-2" /> | ||
| <button |
There was a problem hiding this comment.
From what I'm seeing this was removed because this speed value was not being used anywhere, right? Can we remove the rest of the references to it? Besides legacy implementations (like in the loadMissionFromFile method), you're also using this value in your energy estimates.
There was a problem hiding this comment.
Yes, I did remove it but i'm putting it back. Its too early to take that option off. Users might look for a way to set a different speed and MAVLink commands could be a little advanced for some.
So I have updated the uploadMissionToVehicle function to check if the defaultCruiseSpeed is different than 1 and inject that change_speed into the first wp.
There was a problem hiding this comment.
Nice!
Can you separate this change into a dedicated commit?
There was a problem hiding this comment.
It wasn't ready to review yet, so the change wasn't implemented.
Check again
There was a problem hiding this comment.
The change is a single modification on the mission upload function:
I added:
if (defaultCruiseSpeed.value !== 1) {
missionItemsToUpload[0].commands = [
...missionItemsToUpload[0].commands.filter((cmd) => cmd.command !== MavCmd.MAV_CMD_DO_CHANGE_SPEED),
{
type: MissionCommandType.MAVLINK_NAV_COMMAND,
command: MavCmd.MAV_CMD_DO_CHANGE_SPEED,
param1: 0,
param2: defaultCruiseSpeed.value,
param3: -1,
param4: 0,
},
]
}
Is it okay or you think a separate commit is still needed?
| // Includes speed changes present on mission legs (from wp to wp+1) | ||
| const missionLegsWithSpeed = computed<Array<{ from: number; to: number; distanceMeters: number; speedMps: number }>>( | ||
| () => { | ||
| const anyStore = missionStore as any |
There was a problem hiding this comment.
This anyStore name here (and in other places) does not make sense, but I assume it will go away when you remove the references to the defaultCruiseSpeed variable.
7d2991d to
188f568
Compare
src/libs/utils.ts
Outdated
| * @param {number} deg Angle in degrees. | ||
| * @returns {number} Angle in radians. | ||
| */ | ||
| export const toRad = (deg: number): number => (deg * Math.PI) / 180 |
There was a problem hiding this comment.
We already have the radians() method in this file.
188f568 to
11f9a30
Compare
Agreed! |
11f9a30 to
06d1827
Compare
rafaellehmkuhl
left a comment
There was a problem hiding this comment.
Two functional things:
- Changing the battery type or capacity is not changing the estimates
- Changing the vehicle payload is also not changing the estimates
The blinking bug mentioned in the last review is still there, although it should not hold this PR if you don't find what's going on.
|
BTW the new architecture got much much better. Nice work. |
Hmm.. I'll check what happened |
Maybe for a UX point of view we should only enable the cog icon to known vehicle types that have power consumption already modeled? (1) estimates_changing.mp4 |
Maybe it's macOS specific indeed. And agree, no need to hold it because of that.
I was running a Surface Boat, but from SITL. It should still show up, right?
Definitely! |
06d1827 to
172bd7b
Compare
Check if the vehicleType on vehicleStore is === MavType.MAV_TYPE_SURFACE_BOAT
|
rafaellehmkuhl
left a comment
There was a problem hiding this comment.
As discussed externally, everything is working, is just that the energy estimates for the payload and battery are inaccurate. As this can easily be improved later, I've opened an issue about that and we can go with this PR as it is.
I just marked two code changes that do not belong to this PR that need to be removed before we merge it.
src/stores/mission.ts
Outdated
| currentWaypointAltitude: waypoints[0]?.altitude || 0, | ||
| currentWaypointAltitudeRefType: waypoints[0]?.altitudeReferenceType || AltitudeReferenceType.RELATIVE_TO_HOME, | ||
| defaultCruiseSpeed: 0, | ||
| defaultCruiseSpeed: 1, |
src/views/MissionPlanningView.vue
Outdated
| if (defaultCruiseSpeed.value !== 1) { | ||
| missionItemsToUpload[0].commands = [ | ||
| ...missionItemsToUpload[0].commands.filter((cmd) => cmd.command !== MavCmd.MAV_CMD_DO_CHANGE_SPEED), | ||
| { | ||
| type: MissionCommandType.MAVLINK_NAV_COMMAND, | ||
| command: MavCmd.MAV_CMD_DO_CHANGE_SPEED, | ||
| param1: 0, | ||
| param2: defaultCruiseSpeed.value, | ||
| param3: -1, | ||
| param4: 0, | ||
| }, | ||
| ] | ||
| } | ||
|
|
Signed-off-by: Arturo Manzoli <[email protected]>
Signed-off-by: Arturo Manzoli <[email protected]>
Signed-off-by: Arturo Manzoli <[email protected]>
172bd7b to
b6fa525
Compare
Both problems (panel blinking and energy estimates not changing on different payload and battery capacities) have been addressed. Also removed the code snippets relative to the default cruise speed PR. |
rafaellehmkuhl
left a comment
There was a problem hiding this comment.
It's working perfectly now!
MAV_TYPE_SURFACE_BOAT;DO_CHANGE_SPEEDalso affect time and energy consumption estimates;(1)
Screenshare.-.2025-09-11.9_38_36.AM.mp4
(2)

(3)

(4)

(5)

(6)

(7)

(8)

Closes #1235
Closes #1459