search

openvehicles / Open-Vehicle-Monitoring-System-3

Open Vehicle Monitoring System - Version 3
http:///www.openvehicles.com/
Other
593 stars 225 forks source link

Added support for Energica motorbikes #1059

Closed matthieu-labas closed 1 month ago

matthieu-labas commented 1 month ago

Support for Energica motorbikes : Eva Ribelle, Eva Ego and Experia

dexterbg commented 1 month ago

Matthieu, welcome :-)

Awesome addition, I have a crush on these bikes.

There's a minor name space issue:

m_v_cell_balance = MyMetrics.InitFloat("nrjk.v.b.c.balance", SM_STALE_MIN, 0, Volts);

The vehicle specific namespace for metrics, configs & commands needs to be defined with an "x" prefix. So you can change that to xnrjk, or perhaps a shorter abbrevation like xnr.

Regarding documentation, please be very clear on which models are supported. If this code applies to all current models, still name them explicitly. Probably there also is a name for the drive train system this applies to, if the same system will be used for future Energica bikes, users will know your module will work.

There's also a compiler warning, resulting from the gcc version we currently use not correctly supporting enums stored in bit fields:

In file included from …/vehicle_energica.cpp:52:0:
…/vehicle_energica_pids.h:250:19: warning: 'pid_410_gps_longalt::_fix' is too small to hold all values of 'enum gps_fix'
  gps_fix  _fix  : 2;
                   ^

This has been fixed in gcc version 11, but as we're still stuck with version 5, you need to work around this bug by using unsigned for the _fix member and casting that to type gps_fix in the accessor method.

Regards, Michael

dexterbg commented 1 month ago

I'll still merge this now, so users can begin testing.

matthieu-labas commented 1 month ago

Oh I didn't think people would know about those awesome bikes! :)

I implemented the changes you mentionned:

How should I proceed now? Is the use to send a PR each time I change something?

Anyway, thank you for the support (in both meaning ;-))

dexterbg commented 1 month ago

You're welcome :)

In case a PR has been merged already, you need to create a new one.

Standard procedure for PRs is to create a dedicated local branch in your repository for each PR. That way you can simply push new commits there (by merging or cherry picking) and get a running PR automatically updated, while you still can have unfinished/unrelated work in progress in your "master" branch.