Discussion:
WOST for hm2 drivers
Add Reply
Bertho Stultiens
2017-06-17 07:58:28 UTC
Reply
Permalink
Raw Message
Hi,

The halcompile feature to compile without source tree (WOST) seems to be
nonfunctional for the hm2 driver(s). This is, at least in part, because
the headers required are not copied to the ../include path (from the
Makefile). There are also local include references that may not hold
true when using halcompile.

In principle, it could be made functional by adding all the required
headers to the list in the Makefile including a bit of fiddling in the
hm2 sources.

The biggest problem I see is increased clutter in the include directory.
In principle, you'd like to separate the driver headers into
include/hm2/*. The alternative is to rename the files and prefix them
with, f.ex. hm2_ to make it clear where they belong.

Another detail is that there does not seem to be a consensus in the
drivers about using <> and "" for includes. They do differ in function
and the latter may give problems for WOST.

So, anyway you look at it, it will be some rearranging, where "some"
probably ranges from "arrrgh!" to "sigh". However, I think it is
necessary to fix.


Any thoughts?
--
Greetings Bertho

(disclaimers are disclaimed)
andy pugh
2017-06-17 08:56:42 UTC
Reply
Permalink
Raw Message
Post by Bertho Stultiens
Any thoughts?
Is there any need to compile the hm2 drivers with halcompile?

halcompile is intended for users to compile custom HAL components with, I
don't think that the intention was for it to compile arbitrary lumps of
LinuxCNC.

What _would_ be nice would be if it could compile some of the more
elaborate kinematics files, as users are rather expected to create custom
kins.
--
atp
"A motorcycle is a bicycle with a pandemonium attachment and is designed
for the especial use of mechanical geniuses, daredevils and lunatics."
— George Fitch, Atlanta Constitution Newspaper, 1916
Bertho Stultiens
2017-06-17 09:19:15 UTC
Reply
Permalink
Raw Message
Post by andy pugh
Post by Bertho Stultiens
Any thoughts?
Is there any need to compile the hm2 drivers with halcompile?
halcompile is intended for users to compile custom HAL components with, I
don't think that the intention was for it to compile arbitrary lumps of
LinuxCNC.
Well, the question arises from the forum post (which you directed me to)
while I was hacking the hm2_rpspi driver. It seems that users do try...

I agree that compiling arbitrary pieces of code may not be the primary
goal of halcompile. OTOH, compiling /drivers/ may in fact be a valid use
of halcompile, especially when running uspace.

The use of abstraction levels and interface libraries makes it "easy" to
have a plug-able system. It may be no bad idea to hold that thought
throughout.
Post by andy pugh
What _would_ be nice would be if it could compile some of the more
elaborate kinematics files, as users are rather expected to create custom
kins.
That I have not looked into (yet).
--
Greetings Bertho

(disclaimers are disclaimed)
andy pugh
2017-06-17 09:30:09 UTC
Reply
Permalink
Raw Message
Post by Bertho Stultiens
I agree that compiling arbitrary pieces of code may not be the primary
goal of halcompile. OTOH, compiling /drivers/ may in fact be a valid use
of halcompile, especially when running uspace.
In the specific case of hm2 you can't even begin to understand how to write
a driver without the full source tree anyway, and at that point you might
as well use "make" in the usual way.

The hm2 UART and BSPI interfaces do expose an interface that can be used by
halcompiled drivers written in .comp. Though I don't know if many people
do, and I rather wish I had chosen a different route.
--
atp
"A motorcycle is a bicycle with a pandemonium attachment and is designed
for the especial use of mechanical geniuses, daredevils and lunatics."
— George Fitch, Atlanta Constitution Newspaper, 1916
Jeff Epler
2017-06-17 21:06:37 UTC
Reply
Permalink
Raw Message
Another important consideration is whether the hostmot2 API is stable
enough that we want to make it a part of the public API of LinuxCNC.
That means things like not deliberately making breaking API changes
without notice for a whole stable release cycle; so e.g., if we do this
in 2.8, then want to make a change, we have to still support the same
API during 2.9 and only change it in 2.10; this is a multi-year process,
since we don't even start a stable release cycle on a yearly basis these
days.

Jeff
Bertho Stultiens
2017-06-17 22:39:57 UTC
Reply
Permalink
Raw Message
Post by Jeff Epler
Another important consideration is whether the hostmot2 API is stable
enough that we want to make it a part of the public API of LinuxCNC.
That means things like not deliberately making breaking API changes
without notice for a whole stable release cycle; so e.g., if we do this
in 2.8, then want to make a change, we have to still support the same
API during 2.9 and only change it in 2.10; this is a multi-year process,
since we don't even start a stable release cycle on a yearly basis these
days.
Points well taken (also yours, Andy). I'll leave it as is for now.

That said, there is one small item left and that is cleaning the include
statements (f.ex. "" vs <>) and have it consistent across all source files.
--
Greetings Bertho

(disclaimers are disclaimed)
Jeff Epler
2017-06-17 23:46:56 UTC
Reply
Permalink
Raw Message
.. as a specific example, hostmot2.h defines hostmot2_t. Between 2.7
and master, we added the config.pktuarts and pktuart fields to it. This
is technically API compatible, since the fields were only added; the
meaning of existing fields is not changed. However, it is ABI
incompatible since the size and layout of the structure changed. We've
never tried to be ABI compatible in this way.

Between 2.6 and 2.7, we made an incompatible API change, removing one
API and adding two to replace it:

-int hm2_sserial_check_errors(hostmot2_t *hm2, hm2_sserial_instance_t *inst);
+int hm2_sserial_check_local_errors(hostmot2_t *hm2, hm2_sserial_instance_t *inst);
+int hm2_sserial_check_remote_errors(hostmot2_t *hm2, hm2_sserial_instance_t *inst);

If this had been public API, then we would have needed to provide the
old API too, during the interval that the old API was deprecated.

Jeff

Loading...