AP_RPM: Allow more instances

[magicrub]

Allow AP_RPM instances RPM_MAX_INSTANCES up to 9

[IamPete1]

We really should move to instance logging.

[magicrub]

We really should move to instance logging.

What do you mean?

[IamPete1]

What do you mean?

We currently log the first two instances in a single message, we should move to a log with a instance number and also log health.

ardupilot/libraries/AP_RPM/AP_RPM.cpp

Lines 306 to 311 in baa6bc0

	const struct log_RPM pkt{
	LOG_PACKET_HEADER_INIT(LOG_RPM_MSG),
	time_us : AP_HAL::micros64(),
	rpm1 : rpm1,
	rpm2 : rpm2
	};

Currently this adds the new inertances, but there not logged and not sent back over MAVLink, so there fairly pointless.

[magicrub]

@IamPete1 oh, right. Of course. Well, they're not useless if they're feeding other libraries. I'm not sure if I should adding the logging in this PR or a follow-up. What do you think? It will obviously require some refactoring and such which is quasi-out-of-scope. I'm happy to make follow-up PRs for it. Less excited about rolling it into this one

[magicrub]

I initially had a need for this on a periph but now 2 instances are OK. So, other than that no else has asked for this to my knowledge so I'm ok closing it. However, it's still potentially useful for future stuff and follows the pattern of other libraries. Just need to figure out the logging bit of it but in the case of a periph then the usefulness of implementing logging is questionable anyway.

I'll tag for devcall to get a thumbs up/down on if it's worth perusing as-is and/or with logging changes in this/future PR.

[magicrub]

the periph use-case I have in mind was a single device/hub that handled 4 of CAN -> PWM_ESCs channels with each having an RPM feedback populating an ESC_Telem feedback message (could also populate voltage/current and temperatures as well)

[IamPete1]

I have had to add extra RPM instances in the past, I think adding some more is worthwhile, 9 instances might be a bit much, but two additional instances would be useful.

[tridge]

need to make RPM message an instance with RPM[0].rpm RPM[1].rpm etc

[rmackay9]

Let's make it 4 instances.. that should be enough.

[magicrub]

Changed max to 4 instances and updated logger to be instance basec

Tridge's requests have been addressed.

[rmackay9]

This looks OK to me. It's been tested of course including testing that the logs appear correctly for multiple instances?

[magicrub]

This looks OK to me. It's been tested of course including testing that the logs appear correctly for multiple instances?

The logging needs testing. I'll report back when I've done that

[magicrub]

Would be nice to get another review of this to ensure I correctly implemented the method that was discussed in the last dev call.

[tridge]

just needs test

[peterbarker]

... now needs rebase...

[peterbarker]

I've force-pushed this branch after rebasing it on top of Matt's RPM-logging-improvements branch (which I vastly prefer over moving to WriteStreaming!)

[peterbarker]

Tested the logging in SITL. Works fine.